Re: [PATCH V2 6/6] powerpc/perf/htm: Add documentation for Hardware Trace Macro PMU

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Medium] The documentation states that `perf record` or `perf report -D` generates specific binary output files (`htm.bin.*` and `translation.*`), but this functionality does not exist in the upstream `tools/perf` codebase.
- [Medium] The commit message claims to document `htmdecode` usage and notes on PMU restrictions, but these sections are entirely missing from the documentation text.
- [Low] There are several grammatical errors, typos, and inconsistencies in the newly added documentation text.
--

--- Patch [6]: [PATCH V2 6/6] powerpc/perf/htm: Add documentation for Hardware Trace Macro PMU ---
commit 630c94afa3c07b9d61db47792f5a5e0b40edd107
Author: Athira Rajeev <atrajeev@linux.ibm.com>

powerpc/perf/htm: Add documentation for Hardware Trace Macro PMU

This commit extends the HTM documentation with a new section covering the
perf PMU interface. It describes event configuration, syntax, basic usage
examples, and expected output files.

>     The added documentation covers:
>     
>       - How to open HTM events using perf record, including the event
>         syntax (nodalchipindex, nodeindex, htm_type, cpu=N) and the
>         required AUX buffer size (-m,256).
>     
>       - The two output files produced by perf report:
>           htm.bin.nX.pX.cX     raw bus-trace AUX data
>           translation.nX.pX.cX memory-configuration records
>     
>       - How to pass the output files to htmdecode for trace decoding.
>     
>       - Notes on system-wide collection (-a) vs CPU-pinned collection
>         (-C N) and the one-event-per-target PMU restriction.

[Severity: Medium]
The commit message claims to document htmdecode usage, as well as notes on
system-wide vs CPU-pinned collection and PMU restrictions, but these sections
appear to be missing entirely from the added documentation text. Did these get
dropped during patch iteration?

> diff --git a/Documentation/arch/powerpc/htm.rst b/Documentation/arch/powerpc/htm.rst
> index fcb4eb6306b16..d574fd2225ea5 100644
> --- a/Documentation/arch/powerpc/htm.rst
> +++ b/Documentation/arch/powerpc/htm.rst
[ ... ]
> @@ -94,7 +95,137 @@ This trace file will contain the relevant instruction traces
>  collected during the workload execution. And can be used as
>  input file for trace decoders to understand data.
>  
> -Benefits of using HTM debugfs interface
> +HTM perf interface usage
> +========================
[ ... ]
> +- event: "config:0-27"
> +- htm_type: "config:0-3"
> +- nodeindex: "config:4-11"
> +- nodalchipindex: "config:12-19"
> +- coreindexonchip: "config:20-27"
> +
> +1) nodeindex, nodalchipindex, coreindexonchip: this specifies
> +   which partition to configure the HTM for.
> +2) htmtype: specifies the type of HTM.

[Severity: Low]
This isn't a bug, but should this say htm_type instead of htmtype to match
the parameter list above?

> +
> +Event Syntax
> +------------
> +
> +The event configuration uses named parameters::
> +
> +   htm/nodeindex=N,nodalchipindex=C,coreindexonchip=R,htm_type=T/
> +
> +To open the event on a specific cpu can be specified using::

[Severity: Low]
This isn't a bug, but there is a grammatical error here. Could this be updated
to something like "Opening the event on a specific CPU can be specified
using::"?

> +
> +   htm/nodeindex=N,nodalchipindex=C,coreindexonchip=R,htm_type=T,cpu=x/
> +
> +Where:
> +
> +- N = node index
> +- C = chip index within the node
> +- R = core index on the chip
> +- T = HTM type
> +- x = CPU number
> +
> +Basic Usage Example
> +-------------------
> +
> +To collect HTM trace data for a specific chip:
> +
> +.. code-block:: sh
> +
> +   # perf record -C 1 -e htm/nodalchipindex=2,nodeindex=0,htm_type=1/ <workload>
> +
> +In this example:
> +
> +- ``-C 1``: Collect on CPU 1
> +- ``nodeindex=0``: Target node 0
> +- ``nodalchipindex=2``: Target chip 2 within node 0
> +- ``htm_type=1``: HTM trace type 1
> +
> +.. code-block:: sh
> +
> +   # perf record -m,256 -e htm/coreindexonchip=6,nodalchipindex=0,nodeindex=0,htm_type=2,cpu=16/ -a sleep 1
> +
> +In this example:
> +
> +- ``cpu=16``: Collect on CPU 16
> +- ``nodeindex=0``: Target node 0
> +- ``nodalchipindex=0``: Target chip 0 within node 0
> +- ``coreindexonchip=6``: Target code 6

[Severity: Low]
This isn't a bug, but there's a typo here. Should "Target code" be
"Target core"?

> +- ``htm_type=2``: HTM trace type 2
> +- ``-m,256``: specifies number of mmap pages
> +
> +Running trace collection for multiple targets:
> +
> +.. code-block:: sh
> +
> +   # perf record -m,256 -e htm/nodalchipindex=2,nodeindex=0,htm_type=1,cpu=8/ -e htm/nodalchipindex=1,nodeindex=0,htm_type=1,cpu=9/ -a sleep 1
> +
> +
> +In this example, trace is collected for two events on different target chips
> +
> +Output Files
> +------------
> +
> +After running ``perf record``, the following files are generated:
> +
> +.. code-block:: sh
> +
> +   # ls htm.bin.*
> +   htm.bin.n0.p2.c0 htm.bin.n1.p3.c0  # Binary trace files
> +
> +   # ls translation.*
> +   translation.n0.p2.c0  translation.n1.p3.c0  # Memory configuration files
> +
> +These files contain:
> +
> +- **htm.bin.*** - Raw HTM trace data in binary format
> +- **translation.*** - Memory address translation information for decoding
> +
> +Complete Workflow Example
> +--------------------------
> +
> +Here's a complete example of collecting and analyzing HTM traces:
> +
> +.. code-block:: sh
> +
> +   # Step 1: Collect trace data
> +   perf record -C 1 -e htm/nodalchipindex=2,nodeindex=0,htm_type=1/ sleep 5
> +
> +   # Step 2: Verify output files
> +   perf report -D
> +
> +   ls htm.bin.*        # Binary trace files
> +   ls translation.*    # Memory configuration files

[Severity: Medium]
Does perf report -D natively generate these specific binary output files
(htm.bin.* and translation.*) in the upstream tools/perf codebase?

This documentation seems to describe a non-existent feature in upstream
tools/perf and might rely on out-of-tree tooling that was not included in
this patch series. Should this be updated to reflect only supported features?

> +   ls perf.data        # Perf data file
> +
> +Benefits of using HTM interface

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260720104447.11843-1-atrajeev@linux.ibm.com?part=6