Re: [PATCH v2 0/3] Arm CMN-600 PMU driver

From: "Zidenberg, Tsahi" <tsahee@amazon.com>
To: Robin Murphy <robin.murphy@arm.com>, Will Deacon <will@kernel.org>
Cc: "mark.rutland@arm.com" <mark.rutland@arm.com>,
	"linux-arm-kernel@lists.infradead.org" 
	<linux-arm-kernel@lists.infradead.org>,
	"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
	"Saidi, Ali" <alisaidi@amazon.com>,
	"harb@amperecomputing.com" <harb@amperecomputing.com>,
	"tuanphan@os.amperecomputing.com"
	<tuanphan@os.amperecomputing.com>,
	"james.yang@arm.com" <james.yang@arm.com>,
	"patrik.berglund@arm.com" <patrik.berglund@arm.com>
Subject: Re: [PATCH v2 0/3] Arm CMN-600 PMU driver
Date: Thu, 15 Oct 2020 07:26:06 +0000	[thread overview]
Message-ID: <376E9048-6702-4146-822E-FCF992661B85@amazon.com> (raw)
In-Reply-To: <b86a8e7e-4def-3cd0-d2c1-b9560304cf15@arm.com>

On 14/10/2020, 19:31, "Robin Murphy" <robin.murphy@arm.com> wrote:
    > On Mon, Oct 05, 2020 at 09:16:13AM +0000, Zidenberg, Tsahi wrote:
    >>   By default each event provides an aggregate count over all nodes of the
    >>   given type. To target a specific node, "bynodeid" must be set to 1 and
    >> -"nodeid" to the appropriate value derived from the CMN configuration
    >> -(as defined in the "Node ID Mapping" section of the TRM).
    >> +"nodeid" to the appropriate value derived from the CMN configuration.
    >> +
    >> +The CMN map is available in /sys/kernel/debug/arm-cmn/map.
    >> +A nodeid could be calculated with this formulae:
    >> +  node-id = d | (p << 2) | (y << 3) | (x << (3 + xybits))
    >> +where:
    >> +  x,y,p,d are node location as can be seen in the map
    >> +  xybits is 2 for meshes <= 2*2, and 3 otherwise.

    > Note that I disagree strongly with removing the reference to the
    > documentation that canonically defines the node ID format, especially
    > when the proposal is to replace it with a harder-to-read definition that
    > is also wrong ;)

Would be happy to be corrected :) 

    > If there's actually interest in using the coordinate format rather than
    > just whole ID values, then we should generate format attributes that
    > overlay the appropriate sections of the nodeid field. I haven't tried to
    > implement that so far since it's not entirely trivial and I wasn't sure
    > if anyone would care.

I agree that over 90% of the work with the counters should be with aggregate
values that the driver already handles well by default.
At the next level, there might be a lot to gain from just understanding if the
counter in question is going up across the board, or just for a specific node.
Are there a lot of HNF cache misses in the system, or from one specific HNF?
To answer that question, the developer doesn't need to hold the SOC SPEC,
just to have the list of relevant node-ids per type/counter.
I really like the arm-cmn/map you created. There is still a small missing link
between that map to relevant node-ids for the above. In my opinion, looking
at specific node-ids would be a rare enough task, requiring enough expertise,
that I am comfortable putting a formulae in documentation without actually
modifying API, at least at this stage.

Thank you!
Tsahi.