On 2020-10-05 11:21, Will Deacon wrote:
On Mon, Oct 05, 2020 at 09:16:13AM +0000, Zidenberg, Tsahi wrote:
On 18/09/2020, 16:29, "Robin Murphy" <robin.murphy@xxxxxxx> wrote:
> Here's an update with some very minor cleanup tweaks, plus a proposal
> for some more in-depth debug information. Given that nobody seems to
> have any significant complaints about the interface, I assume we're all
> happy to merge the basic driver as-is and fix anything later if needed.
>
> Robin.
Agree with merging the driver as-is.
Tested both the updated driver and the debugfs on Graviton2 (ACPI).
Both map and initial numbers seem right.
Tested-by: Tsahi Zidenberg <tsahee@xxxxxxxxxx>
May I offer a small addition to the documentation?
Something along these lines:
--- a/Documentation/admin-guide/perf/arm-cmn.rst
+++ b/Documentation/admin-guide/perf/arm-cmn.rst
@@ -41,8 +41,14 @@ specified by "occupid".
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 ;)
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 already queued the driver, but if you send this as a stand-alone patch (on
top of the arm64 for-next/perf branch) then I'm happy to take it.
Ha, so much for taking my holiday after -rc6 and expecting to come back
to chasing this up at -rc1... cheers Will, that was a very nice surprise
to return to!
Robin.
