From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751394AbdFFKIp (ORCPT ); Tue, 6 Jun 2017 06:08:45 -0400 Received: from merlin.infradead.org ([205.233.59.134]:55492 "EHLO merlin.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751274AbdFFKIo (ORCPT ); Tue, 6 Jun 2017 06:08:44 -0400 Date: Tue, 6 Jun 2017 12:08:22 +0200 From: Peter Zijlstra To: Andi Kleen Cc: acme@kernel.org, linux-kernel@vger.kernel.org, jolsa@kernel.org, Andi Kleen , Stephane Eranian Subject: Re: [PATCH 2/6] perf/x86: Fix data source decoding for Skylake Message-ID: <20170606100822.fds5qmgimz74jfja@hirez.programming.kicks-ass.net> References: <20170605224838.11759-1-andi@firstfloor.org> <20170605224838.11759-2-andi@firstfloor.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20170605224838.11759-2-andi@firstfloor.org> User-Agent: NeoMutt/20170113 (1.7.2) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, Jun 05, 2017 at 03:48:34PM -0700, Andi Kleen wrote: > +void __init intel_pmu_pebs_data_source_skl(void) > +{ > + pebs_data_source[0x08] = OP_LH | P(LVLX, L4) | P(SNOOP, HIT); > + pebs_data_source[0x09] = OP_LH | P(LVLX, L4) | P(LVLX, REMOTE) | P(SNOOP, HIT); > + pebs_data_source[0x0b] = OP_LH | P(LVLX, RAM) | P(LVLX, REMOTE) | P(SNOOP, NONE); > + pebs_data_source[0x0c] = OP_LH | P(LVL, NA) | P(LVLX, REMOTE) | P(SNOOPX, FWD); > + pebs_data_source[0x0d] = OP_LH | P(LVL, NA) | P(LVLX, REMOTE) | P(SNOOP, HITM); > +} Not too happy about that.. P(LVLX, L4) | P(LVLX, REMOTE) reads like something that should be PERF_MEM_LVL_REM_CCE1 or something and P(LVLX, RAM) | P(LVLX, REMOTE) Should certainly be PERF_MEM_LVL_REM_RAM1 This new generic 'REMOTE' has too much overlap with the existing things. I realize the pickle you're in, but urgh.. Stephane? > diff --git a/include/uapi/linux/perf_event.h b/include/uapi/linux/perf_event.h > index b1c0b187acfe..4b5deeada34b 100644 > --- a/include/uapi/linux/perf_event.h > +++ b/include/uapi/linux/perf_event.h > @@ -931,14 +931,18 @@ union perf_mem_data_src { > mem_snoop:5, /* snoop mode */ > mem_lock:2, /* lock instr */ > mem_dtlb:7, /* tlb access */ > - mem_rsvd:31; > + mem_lvlx:8, /* memory hierarchy level, ext */ > + mem_snoopx:2, /* snoop mode, ext */ > + mem_rsvd:21; > }; > }; > #elif defined(__BIG_ENDIAN_BITFIELD) > union perf_mem_data_src { > __u64 val; > struct { > - __u64 mem_rsvd:31, > + __u64 mem_rsvd:21, > + mem_snoopx:2, /* snoop mode, ext */ > + mem_lvlx:8, /* memory hierarchy level, ext */ > mem_dtlb:7, /* tlb access */ > mem_lock:2, /* lock instr */ > mem_snoop:5, /* snoop mode */ > @@ -975,6 +979,13 @@ union perf_mem_data_src { > #define PERF_MEM_LVL_UNC 0x2000 /* Uncached memory */ > #define PERF_MEM_LVL_SHIFT 5 > > +#define PERF_MEM_LVLX_REMOTE 0x01 /* Remote */ > +#define PERF_MEM_LVLX_L4 0x02 /* L4 */ > +#define PERF_MEM_LVLX_RAM 0x04 /* Ram */ > +/* 5 free */ > + > +#define PERF_MEM_LVLX_SHIFT 33 > + > /* snoop mode */ > #define PERF_MEM_SNOOP_NA 0x01 /* not available */ > #define PERF_MEM_SNOOP_NONE 0x02 /* no snoop */ > @@ -983,6 +994,10 @@ union perf_mem_data_src { > #define PERF_MEM_SNOOP_HITM 0x10 /* snoop hit modified */ > #define PERF_MEM_SNOOP_SHIFT 19 > > +#define PERF_MEM_SNOOPX_FWD 0x01 /* forward */ > +/* 1 free */ > +#define PERF_MEM_SNOOPX_SHIFT 41 > + > /* locked instruction */ > #define PERF_MEM_LOCK_NA 0x01 /* not available */ > #define PERF_MEM_LOCK_LOCKED 0x02 /* locked transaction */ > -- > 2.9.4 >