From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 8BC43C433EF for ; Sat, 26 Mar 2022 13:53:24 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=GTOedJxiZhtzvu0nYqjnX/odZmZSz8oVjE91qgOpmwg=; b=EB7+OPeBVEYCcS bLQbY8vIX8aQfBWy19ndGr7ntdLamJpkCD5O5dnrNW9AjS+N0dhRi9q8JpfTheB/VFHh5HETtFEDI tLKmhAW4gtsuXZJ84LRwLivs7tpvpQC3IDd9w40YiiXuNluRm+yBH/NxAKB594uZWofchk+HoUOev oUEoWiBDd7ViZDpPLUD0SqRNw7P9nyY1QAila6yKMu2chjYc7Y34RQVZAzClFTMYYRHJZZAf02d+9 VVHBFkO4wF38K9duRd1dmHoj9UoB+QdwNUFPkbYvxcwXS/iVpdrJEChq1Uyk9ZDiCuALYmq10xgaL 4jL+2NTaoL3UiBEP2c3A==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1nY6pv-004KPb-Tu; Sat, 26 Mar 2022 13:52:12 +0000 Received: from casper.infradead.org ([2001:8b0:10b:1236::1]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1nY6ps-004KPI-T4 for linux-arm-kernel@bombadil.infradead.org; Sat, 26 Mar 2022 13:52:09 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=casper.20170209; h=In-Reply-To:Content-Type:MIME-Version: References:Message-ID:Subject:Cc:To:From:Date:Sender:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description; bh=ZNHK80AnJw2rgEYob/bpP76p4mcIEU/H1B+eXPsKSWI=; b=BJ4TdLGDhSVo9SLfXqD6YIsbpN 7QTXOUxc6If3Yyhtzq5eRM7FSi/A2gzXlK1svmiPu+RZBOfgn4PS9Hla5sPugUAr/yCBnoZY0oShM CMliYPkX3/IC3f++lOgkaE0TMGQ9oqedePtVlQlMVtBSU73ic7NuR55CnW04FD4N87qBZUR65AHKO HcKwQAvDKaWWT/UJFuVWT4d5alxmd0TacAVbKWGm7c24o7of3pambjZYnj7iZJzhi+w+B8czkqZb7 /oYb5TihmvfVBDPXnFnf8JfNTWSQ9XsLCUAoN7LTdbzGYLQj1MuqmQCHgwlH5EHgqXB8VkqHX15mM yRui0xvw==; Received: from [187.19.238.43] (helo=quaco.ghostprotocols.net) by casper.infradead.org with esmtpsa (Exim 4.94.2 #2 (Red Hat Linux)) id 1nY6pp-00FI6s-Cf; Sat, 26 Mar 2022 13:52:05 +0000 Received: by quaco.ghostprotocols.net (Postfix, from userid 1000) id F0B4440407; Sat, 26 Mar 2022 10:52:01 -0300 (-03) Date: Sat, 26 Mar 2022 10:52:01 -0300 From: Arnaldo Carvalho de Melo To: Leo Yan Cc: Ali Saidi , linux-kernel@vger.kernel.org, linux-perf-users@vger.kernel.org, linux-arm-kernel@lists.infradead.org, german.gomez@arm.com, benh@kernel.crashing.org, Nick.Forrington@arm.com, alexander.shishkin@linux.intel.com, andrew.kilroy@arm.com, james.clark@arm.com, john.garry@huawei.com, jolsa@kernel.org, kjain@linux.ibm.com, lihuafei1@huawei.com, mark.rutland@arm.com, mathieu.poirier@linaro.org, mingo@redhat.com, namhyung@kernel.org, peterz@infradead.org, will@kernel.org Subject: Re: [PATCH v4 2/4] perf arm-spe: Use SPE data source for neoverse cores Message-ID: References: <20220324183323.31414-1-alisaidi@amazon.com> <20220324183323.31414-3-alisaidi@amazon.com> <20220326134754.GD20556@leoy-ThinkPad-X240s> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20220326134754.GD20556@leoy-ThinkPad-X240s> X-Url: http://acmel.wordpress.com X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org Em Sat, Mar 26, 2022 at 09:47:54PM +0800, Leo Yan escreveu: > Hi Ali, German, > > On Thu, Mar 24, 2022 at 06:33:21PM +0000, Ali Saidi wrote: > > [...] > > > +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; > > Firstly, apologize that I didn't give clear idea when Ali sent patch sets > v2 and v3. Ok, removing this as well. Thanks for reviewing. - Arnaldo > IMHO, we need to consider two kinds of information which can guide us > for a reliable implementation. The first thing is to summarize the data > source configuration for x86 PEBS, we can dive in more details for this > part; the second thing is we can refer to the AMBA architecture document > ARM IHI 0050E.b, section 11.1.2 'Crossing a chip-to-chip interface' and > its sub section 'Suggested DataSource values', which would help us > much for mapping the cache topology to Arm SPE data source. > > As a result, I summarized the data source configurations for PEBS and > Arm SPE Neoverse in the spreadsheet: > https://docs.google.com/spreadsheets/d/11YmjG0TyRjH7IXgvsREFgTg3AVtxh2dvLloRK1EdNjU/edit?usp=sharing > > Please see below comments. > > > + 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; > > + break; > > I think we need to set the field 'mem_snoop' for L1 cache hit: > > data_src->mem_snoop = PERF_MEM_SNOOP_NONE; > > For L1 cache hit, it doesn't involve snooping. > > > + case ARM_SPE_NV_L2: > > + data_src->mem_lvl = PERF_MEM_LVL_HIT; > > + data_src->mem_lvl_num = PERF_MEM_LVLNUM_L2; > > + break; > > Ditto: > > data_src->mem_snoop = PERF_MEM_SNOOP_NONE; > > > + case ARM_SPE_NV_PEER_CORE: > > + data_src->mem_lvl = PERF_MEM_LVL_HIT; > > + data_src->mem_snoop = PERF_MEM_SNOOP_HITM; > > + data_src->mem_lvl_num = PERF_MEM_LVLNUM_ANY_CACHE; > > Peer core contains its local L1 cache, so I think we can set the > memory level L1 to indicate this case. > > For this data source type and below types, though they indicate > the snooping happens, but it doesn't mean the data in the cache line > is in 'modified' state. If set flag PERF_MEM_SNOOP_HITM, I personally > think this will mislead users when report the result. > > I prefer we set below fields for ARM_SPE_NV_PEER_CORE: > > data_src->mem_lvl = PERF_MEM_LVL_HIT | PERF_MEM_LVL_L1; > data_src->mem_snoop = PERF_MEM_SNOOP_HIT; > data_src->mem_lvl_num = PERF_MEM_LVLNUM_L1; > > > + break; > > + /* > > + * We don't know if this is L1, L2 but we do know it was a cache-2-cache > > + * transfer, so set SNOOP_HITM > > + */ > > + case ARM_SPE_NV_LCL_CLSTR: > > For ARM_SPE_NV_LCL_CLSTR, it fetches the data from the shared cache in > the cluster level, it should happen in L2 cache: > > data_src->mem_lvl = PERF_MEM_LVL_HIT | PERF_MEM_LVL_L2; > data_src->mem_snoop = PERF_MEM_SNOOP_HIT; > data_src->mem_lvl_num = PERF_MEM_LVLNUM_L2; > > > + case ARM_SPE_NV_PEER_CLSTR: > > + data_src->mem_lvl = PERF_MEM_LVL_HIT; > > + data_src->mem_snoop = PERF_MEM_SNOOP_HITM; > > + data_src->mem_lvl_num = PERF_MEM_LVLNUM_ANY_CACHE; > > + break; > > This type can snoop from L1 or L2 cache in the peer cluster, so it > makes sense to set cache level as PERF_MEM_LVLNUM_ANY_CACHE. But here > should use the snoop type PERF_MEM_SNOOP_HIT, so: > > data_src->mem_lvl = PERF_MEM_LVL_HIT > data_src->mem_snoop = PERF_MEM_SNOOP_HIT; > data_src->mem_lvl_num = PERF_MEM_LVLNUM_ANY_CACHE; > > > + /* > > + * 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; > > + break; > > data_src->mem_lvl = PERF_MEM_LVL_HIT | PERF_MEM_LVL_L3; > data_src->mem_snoop = PERF_MEM_SNOOP_HIT; > data_src->mem_lvl_num = PERF_MEM_LVLNUM_L3; > > > + /* > > + * 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_HITM; > > + data_src->mem_remote = PERF_MEM_REMOTE_REMOTE; > > + break; > > The type ARM_SPE_NV_REMOTE is a snooping operation and it can happen > in any cache levels in remote chip: > > data_src->mem_lvl = PERF_MEM_LVL_HIT; > data_src->mem_snoop = PERF_MEM_SNOOP_HIT; > data_src->mem_remote = PERF_MEM_REMOTE_REMOTE; > data_src->mem_lvl_num = PERF_MEM_LVLNUM_ANY_CACHE; > > > + case ARM_SPE_NV_DRAM: > > + data_src->mem_lvl = PERF_MEM_LVL_HIT; > > + data_src->mem_lvl_num = PERF_MEM_LVLNUM_RAM; > > + break; > > We can set snoop as PERF_MEM_SNOOP_MISS for DRAM data source: > > data_src->mem_lvl = PERF_MEM_LVL_HIT; > data_src->mem_snoop = PERF_MEM_SNOOP_MISS; > data_src->mem_lvl_num = PERF_MEM_LVLNUM_RAM; > > The rest of this patch looks good to me. > > Thanks, > Leo > > > + 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 +525,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 +1262,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 +1283,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; > > > > spe->timeless_decoding = arm_spe__is_timeless_decoding(spe); > > > > -- > > 2.32.0 > > -- - Arnaldo _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel