[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20220420084237.GD843168@leoy-ThinkPad-X240s>
Date: Wed, 20 Apr 2022 16:42:37 +0800
From: Leo Yan <leo.yan@...aro.org>
To: Ali Saidi <alisaidi@...zon.com>
Cc: linux-kernel@...r.kernel.org, linux-perf-users@...r.kernel.org,
linux-arm-kernel@...ts.infradead.org, german.gomez@....com,
acme@...nel.org, benh@...nel.crashing.org, Nick.Forrington@....com,
alexander.shishkin@...ux.intel.com, andrew.kilroy@....com,
james.clark@....com, john.garry@...wei.com, jolsa@...nel.org,
kjain@...ux.ibm.com, lihuafei1@...wei.com, mark.rutland@....com,
mathieu.poirier@...aro.org, mingo@...hat.com, namhyung@...nel.org,
peterz@...radead.org, will@...nel.org
Subject: Re: [PATCH v5 4/5] perf arm-spe: Use SPE data source for neoverse
cores
On Fri, Apr 08, 2022 at 07:53:43PM +0000, Ali Saidi wrote:
> When synthesizing data from SPE, augment the type with source information
> for Arm Neoverse cores. The field is IMPLDEF but the Neoverse cores all use
> the same encoding. I can't find encoding information for any other SPE
> implementations to unify their choices with Arm's thus that is left for
> future work.
>
> This change populates the mem_lvl_num for Neoverse cores instead of the
> deprecated mem_lvl namespace.
>
> Signed-off-by: Ali Saidi <alisaidi@...zon.com>
> ---
> .../util/arm-spe-decoder/arm-spe-decoder.c | 1 +
> .../util/arm-spe-decoder/arm-spe-decoder.h | 12 ++
> tools/perf/util/arm-spe.c | 127 ++++++++++++++++--
> 3 files changed, 126 insertions(+), 14 deletions(-)
>
> diff --git a/tools/perf/util/arm-spe-decoder/arm-spe-decoder.c b/tools/perf/util/arm-spe-decoder/arm-spe-decoder.c
> index 5e390a1a79ab..091987dd3966 100644
> --- a/tools/perf/util/arm-spe-decoder/arm-spe-decoder.c
> +++ b/tools/perf/util/arm-spe-decoder/arm-spe-decoder.c
> @@ -220,6 +220,7 @@ static int arm_spe_read_record(struct arm_spe_decoder *decoder)
>
> break;
> case ARM_SPE_DATA_SOURCE:
> + decoder->record.source = payload;
> break;
> case ARM_SPE_BAD:
> break;
> 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 69b31084d6be..46a61df1145b 100644
> --- a/tools/perf/util/arm-spe-decoder/arm-spe-decoder.h
> +++ b/tools/perf/util/arm-spe-decoder/arm-spe-decoder.h
> @@ -29,6 +29,17 @@ enum arm_spe_op_type {
> ARM_SPE_ST = 1 << 1,
> };
>
> +enum arm_spe_neoverse_data_source {
> + ARM_SPE_NV_L1D = 0x0,
> + ARM_SPE_NV_L2 = 0x8,
> + ARM_SPE_NV_PEER_CORE = 0x9,
> + ARM_SPE_NV_LOCAL_CLUSTER = 0xa,
> + ARM_SPE_NV_SYS_CACHE = 0xb,
> + ARM_SPE_NV_PEER_CLUSTER = 0xc,
> + ARM_SPE_NV_REMOTE = 0xd,
> + ARM_SPE_NV_DRAM = 0xe,
> +};
> +
> struct arm_spe_record {
> enum arm_spe_sample_type type;
> int err;
> @@ -40,6 +51,7 @@ struct arm_spe_record {
> u64 virt_addr;
> u64 phys_addr;
> u64 context_id;
> + u16 source;
> };
>
> struct arm_spe_insn;
> diff --git a/tools/perf/util/arm-spe.c b/tools/perf/util/arm-spe.c
> index d2b64e3f588b..a20285cf98e3 100644
> --- a/tools/perf/util/arm-spe.c
> +++ b/tools/perf/util/arm-spe.c
> @@ -34,6 +34,7 @@
> #include "arm-spe-decoder/arm-spe-decoder.h"
> #include "arm-spe-decoder/arm-spe-pkt-decoder.h"
>
> +#include "../../arch/arm64/include/asm/cputype.h"
> #define MAX_TIMESTAMP (~0ULL)
>
> struct arm_spe {
> @@ -45,6 +46,7 @@ struct arm_spe {
> struct perf_session *session;
> struct machine *machine;
> u32 pmu_type;
> + u64 midr;
>
> struct perf_tsc_conversion tc;
>
> @@ -399,33 +401,127 @@ static bool arm_spe__is_memory_event(enum arm_spe_sample_type type)
> return false;
> }
>
> -static u64 arm_spe__synth_data_source(const struct arm_spe_record *record)
> +static const struct midr_range neoverse_spe[] = {
> + MIDR_ALL_VERSIONS(MIDR_NEOVERSE_N1),
> + MIDR_ALL_VERSIONS(MIDR_NEOVERSE_N2),
> + MIDR_ALL_VERSIONS(MIDR_NEOVERSE_V1),
> + {},
> +};
> +
> +
> +static void arm_spe__synth_data_source_neoverse(const struct arm_spe_record *record,
> + union perf_mem_data_src *data_src)
> {
> - union perf_mem_data_src data_src = { 0 };
> + /*
> + * Even though four levels of cache hierarchy are possible, no known
> + * production Neoverse systems currently include more than three levels
> + * so for the time being we assume three exist. If a production system
> + * is built with four the this function would have to be changed to
> + * detect the number of levels for reporting.
> + */
>
> - if (record->op == ARM_SPE_LD)
> - data_src.mem_op = PERF_MEM_OP_LOAD;
> - else
> - data_src.mem_op = PERF_MEM_OP_STORE;
> + /*
> + * We have no data on the hit level or data source for stores in the
> + * Neoverse SPE records.
> + */
> + if (record->op & ARM_SPE_ST) {
> + data_src->mem_lvl = PERF_MEM_LVL_NA;
> + data_src->mem_lvl_num = PERF_MEM_LVLNUM_ANY_CACHE;
> + data_src->mem_snoop = PERF_MEM_SNOOP_NA;
> + return;
> + }
> +
> +
Redundant new line.
> + switch (record->source) {
> + case ARM_SPE_NV_L1D:
> + data_src->mem_lvl = PERF_MEM_LVL_HIT;
> + data_src->mem_lvl_num = PERF_MEM_LVLNUM_L1;
> + data_src->mem_snoop = PERF_MEM_SNOOP_NONE;
> + break;
> + case ARM_SPE_NV_L2:
> + data_src->mem_lvl = PERF_MEM_LVL_HIT;
> + data_src->mem_lvl_num = PERF_MEM_LVLNUM_L2;
> + data_src->mem_snoop = PERF_MEM_SNOOP_NONE;
> + break;
> + case ARM_SPE_NV_PEER_CORE:
> + data_src->mem_lvl = PERF_MEM_LVL_HIT;
> + data_src->mem_snoopx = PERF_MEM_SNOOPX_PEER;
> + data_src->mem_lvl_num = PERF_MEM_LVLNUM_L2;
> + break;
> + /*
> + * We don't know if this is L1, L2 but we do know it was a cache-2-cache
> + * transfer, so set SNOOPX_PEER
> + */
> + case ARM_SPE_NV_LOCAL_CLUSTER:
> + case ARM_SPE_NV_PEER_CLUSTER:
> + data_src->mem_lvl = PERF_MEM_LVL_HIT;
> + data_src->mem_snoopx = PERF_MEM_SNOOPX_PEER;
As a side topic, it's better to use a new patch to dump snooping flag
PERF_MEM_SNOOPX_PEER, some code like below:
diff --git a/tools/perf/util/mem-events.c b/tools/perf/util/mem-events.c
index f8f234251f92..66d44280a4ea 100644
--- a/tools/perf/util/mem-events.c
+++ b/tools/perf/util/mem-events.c
@@ -410,6 +410,11 @@ static const char * const snoop_access[] = {
"HitM",
};
+static const char * const snoopx_access[] = {
+ "Fwd",
+ "Peer",
+};
+
int perf_mem__snp_scnprintf(char *out, size_t sz, struct mem_info *mem_info)
{
size_t i, l = 0;
@@ -430,13 +435,18 @@ int perf_mem__snp_scnprintf(char *out, size_t sz, struct mem_info *mem_info)
}
l += scnprintf(out + l, sz - l, snoop_access[i]);
}
- if (mem_info &&
- (mem_info->data_src.mem_snoopx & PERF_MEM_SNOOPX_FWD)) {
+
+ if (mem_info)
+ m = mem_info->data_src.mem_snoopx;
+
+ for (i = 0; m && i < ARRAY_SIZE(snoopx_access); i++, m >>= 1) {
+ if (!(m & 0x1))
+ continue;
if (l) {
strcat(out, " or ");
l += 4;
}
- l += scnprintf(out + l, sz - l, "Fwd");
+ l += scnprintf(out + l, sz - l, snoopx_access[i]);
}
> + data_src->mem_lvl_num = PERF_MEM_LVLNUM_L3;
> + break;
> + /*
> + * System cache is assumed to be L3
> + */
> + case ARM_SPE_NV_SYS_CACHE:
> + data_src->mem_lvl = PERF_MEM_LVL_HIT;
> + data_src->mem_lvl_num = PERF_MEM_LVLNUM_L3;
> + data_src->mem_snoop = PERF_MEM_SNOOP_HIT;
> + break;
> + /*
> + * We don't know what level it hit in, except it came from the other
> + * socket
> + */
> + case ARM_SPE_NV_REMOTE:
> + data_src->mem_snoop = PERF_MEM_SNOOP_HIT;
> + data_src->mem_remote = PERF_MEM_REMOTE_REMOTE;
> + data_src->mem_snoop = PERF_MEM_SNOOP_NA;
> + break;
> + case ARM_SPE_NV_DRAM:
> + data_src->mem_lvl = PERF_MEM_LVL_HIT;
> + data_src->mem_lvl_num = PERF_MEM_LVLNUM_RAM;
> + data_src->mem_snoop = PERF_MEM_SNOOP_NONE;
> + break;
> + default:
> + break;
> + }
> +}
>
> +static void arm_spe__synth_data_source_generic(const struct arm_spe_record *record,
> + union perf_mem_data_src *data_src)
> +{
> if (record->type & (ARM_SPE_LLC_ACCESS | ARM_SPE_LLC_MISS)) {
> - data_src.mem_lvl = PERF_MEM_LVL_L3;
> + data_src->mem_lvl = PERF_MEM_LVL_L3;
>
> if (record->type & ARM_SPE_LLC_MISS)
> - data_src.mem_lvl |= PERF_MEM_LVL_MISS;
> + data_src->mem_lvl |= PERF_MEM_LVL_MISS;
> else
> - data_src.mem_lvl |= PERF_MEM_LVL_HIT;
> + data_src->mem_lvl |= PERF_MEM_LVL_HIT;
> } else if (record->type & (ARM_SPE_L1D_ACCESS | ARM_SPE_L1D_MISS)) {
> - data_src.mem_lvl = PERF_MEM_LVL_L1;
> + data_src->mem_lvl = PERF_MEM_LVL_L1;
>
> if (record->type & ARM_SPE_L1D_MISS)
> - data_src.mem_lvl |= PERF_MEM_LVL_MISS;
> + data_src->mem_lvl |= PERF_MEM_LVL_MISS;
> else
> - data_src.mem_lvl |= PERF_MEM_LVL_HIT;
> + data_src->mem_lvl |= PERF_MEM_LVL_HIT;
> }
>
> if (record->type & ARM_SPE_REMOTE_ACCESS)
> - data_src.mem_lvl |= PERF_MEM_LVL_REM_CCE1;
> + data_src->mem_lvl |= PERF_MEM_LVL_REM_CCE1;
> +}
> +
> +static u64 arm_spe__synth_data_source(const struct arm_spe_record *record, u64 midr)
> +{
> + union perf_mem_data_src data_src = { 0 };
> + bool is_neoverse = is_midr_in_range(midr, neoverse_spe);
> +
> + if (record->op & ARM_SPE_LD)
> + data_src.mem_op = PERF_MEM_OP_LOAD;
> + else
> + data_src.mem_op = PERF_MEM_OP_STORE;
> +
> + if (is_neoverse)
> + arm_spe__synth_data_source_neoverse(record, &data_src);
> + else
> + arm_spe__synth_data_source_generic(record, &data_src);
>
> if (record->type & (ARM_SPE_TLB_ACCESS | ARM_SPE_TLB_MISS)) {
> data_src.mem_dtlb = PERF_MEM_TLB_WK;
> @@ -446,7 +542,7 @@ static int arm_spe_sample(struct arm_spe_queue *speq)
> u64 data_src;
> int err;
>
> - data_src = arm_spe__synth_data_source(record);
> + data_src = arm_spe__synth_data_source(record, spe->midr);
>
> if (spe->sample_flc) {
> if (record->type & ARM_SPE_L1D_MISS) {
> @@ -1183,6 +1279,8 @@ int arm_spe_process_auxtrace_info(union perf_event *event,
> struct perf_record_auxtrace_info *auxtrace_info = &event->auxtrace_info;
> size_t min_sz = sizeof(u64) * ARM_SPE_AUXTRACE_PRIV_MAX;
> struct perf_record_time_conv *tc = &session->time_conv;
> + const char *cpuid = perf_env__cpuid(session->evlist->env);
> + u64 midr = strtol(cpuid, NULL, 16);
> struct arm_spe *spe;
> int err;
>
> @@ -1202,6 +1300,7 @@ int arm_spe_process_auxtrace_info(union perf_event *event,
> spe->machine = &session->machines.host; /* No kvm support */
> spe->auxtrace_type = auxtrace_info->type;
> spe->pmu_type = auxtrace_info->priv[ARM_SPE_PMU_TYPE];
> + spe->midr = midr;
Except the redundant line, the patch is good for me and I tested it at
my side:
Reviewed-by: Leo Yan <leo.yan@...aro.org>
Tested-by: Leo Yan <leo.yan@...aro.org>
>
> spe->timeless_decoding = arm_spe__is_timeless_decoding(spe);
>
> --
> 2.32.0
>
Powered by blists - more mailing lists