[v9,7/8] docs: trace: Add HiSilicon PTT device driver documentation

Document the introduction and usage of HiSilicon PTT device driver.

Signed-off-by: Yicong Yang <yangyicong@hisilicon.com>
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
 Documentation/trace/hisi-ptt.rst | 307 +++++++++++++++++++++++++++++++
 Documentation/trace/index.rst    |   1 +
 2 files changed, 308 insertions(+)
 create mode 100644 Documentation/trace/hisi-ptt.rst

Hi,

I have started looking at this set.

On Mon, Jun 06, 2022 at 07:55:54PM +0800, Yicong Yang wrote:
> Document the introduction and usage of HiSilicon PTT device driver.
> 
> Signed-off-by: Yicong Yang <yangyicong@hisilicon.com>
> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> ---
>  Documentation/trace/hisi-ptt.rst | 307 +++++++++++++++++++++++++++++++
>  Documentation/trace/index.rst    |   1 +

The "get_maintainer" script clearly indicates that Jonathan Corbet maintains the
Documentation directory and yet he is not CC'ed on this patch, nor is the
linux-doc mainling list.  As such, it would not be possible to merge this
patchset.

>  2 files changed, 308 insertions(+)
>  create mode 100644 Documentation/trace/hisi-ptt.rst
> 
> diff --git a/Documentation/trace/hisi-ptt.rst b/Documentation/trace/hisi-ptt.rst
> new file mode 100644
> index 000000000000..0a3112244d40
> --- /dev/null
> +++ b/Documentation/trace/hisi-ptt.rst
> @@ -0,0 +1,307 @@
> +.. SPDX-License-Identifier: GPL-2.0
> +
> +======================================
> +HiSilicon PCIe Tune and Trace device
> +======================================
> +
> +Introduction
> +============
> +
> +HiSilicon PCIe tune and trace device (PTT) is a PCIe Root Complex
> +integrated Endpoint (RCiEP) device, providing the capability
> +to dynamically monitor and tune the PCIe link's events (tune),
> +and trace the TLP headers (trace). The two functions are independent,
> +but is recommended to use them together to analyze and enhance the
> +PCIe link's performance.
> +
> +On Kunpeng 930 SoC, the PCIe Root Complex is composed of several
> +PCIe cores. Each PCIe core includes several Root Ports and a PTT
> +RCiEP, like below. The PTT device is capable of tuning and
> +tracing the links of the PCIe core.
> +::
> +
> +          +--------------Core 0-------+
> +          |       |       [   PTT   ] |
> +          |       |       [Root Port]---[Endpoint]
> +          |       |       [Root Port]---[Endpoint]
> +          |       |       [Root Port]---[Endpoint]
> +    Root Complex  |------Core 1-------+
> +          |       |       [   PTT   ] |
> +          |       |       [Root Port]---[ Switch ]---[Endpoint]
> +          |       |       [Root Port]---[Endpoint] `-[Endpoint]
> +          |       |       [Root Port]---[Endpoint]
> +          +---------------------------+
> +
> +The PTT device driver registers one PMU device for each PTT device.
> +The name of each PTT device is composed of 'hisi_ptt' prefix with
> +the id of the SICL and the Core where it locates. The Kunpeng 930
> +SoC encapsulates multiple CPU dies (SCCL, Super CPU Cluster) and
> +IO dies (SICL, Super I/O Cluster), where there's one PCIe Root
> +Complex for each SICL.
> +::
> +
> +    /sys/devices/hisi_ptt<sicl_id>_<core_id>

All entries added to sysfs should have corresponding documentation.  See [1] and
[2] for details and [3] for an example.

[1]. https://elixir.bootlin.com/linux/latest/source/Documentation/ABI/README
[2]. https://elixir.bootlin.com/linux/latest/source/Documentation/ABI/testing
[3]. https://elixir.bootlin.com/linux/latest/source/Documentation/ABI/testing/sysfs-bus-coresight-devices-etm4x

> +
> +Tune
> +====
> +
> +PTT tune is designed for monitoring and adjusting PCIe link parameters (events).
> +Currently we support events in 4 classes. The scope of the events
> +covers the PCIe core to which the PTT device belongs.
> +
> +Each event is presented as a file under $(PTT PMU dir)/tune, and
> +a simple open/read/write/close cycle will be used to tune the event.
> +::
> +
> +    $ cd /sys/devices/hisi_ptt<sicl_id>_<core_id>/tune
> +    $ ls
> +    qos_tx_cpl    qos_tx_np    qos_tx_p
> +    tx_path_rx_req_alloc_buf_level
> +    tx_path_tx_req_alloc_buf_level

These look overly long... How about watermark_rx and watermark_tx?

> +    $ cat qos_tx_dp
> +    1
> +    $ echo 2 > qos_tx_dp
> +    $ cat qos_tx_dp
> +    2
> +
> +Current value (numerical value) of the event can be simply read
> +from the file, and the desired value written to the file to tune.
> +
> +1. Tx path QoS control
> +------------------------
> +
> +The following files are provided to tune the QoS of the tx path of
> +the PCIe core.
> +
> +- qos_tx_cpl: weight of Tx completion TLPs
> +- qos_tx_np: weight of Tx non-posted TLPs
> +- qos_tx_p: weight of Tx posted TLPs
> +
> +The weight influences the proportion of certain packets on the PCIe link.
> +For example, for the storage scenario, increase the proportion
> +of the completion packets on the link to enhance the performance as
> +more completions are consumed.
> +
> +The available tune data of these events is [0, 1, 2].
> +Writing a negative value will return an error, and out of range
> +values will be converted to 2. Note that the event value just
> +indicates a probable level, but is not precise.
> +
> +2. Tx path buffer control
> +-------------------------
> +
> +Following files are provided to tune the buffer of tx path of the PCIe core.
> +
> +- tx_path_rx_req_alloc_buf_level: watermark of Rx requested
> +- tx_path_tx_req_alloc_buf_level: watermark of Tx requested
> +
> +These events influence the watermark of the buffer allocated for each
> +type. Rx means the inbound while Tx means outbound. The packets will
> +be stored in the buffer first and then transmitted either when the
> +watermark reached or when timed out. For a busy direction, you should
> +increase the related buffer watermark to avoid frequently posting and
> +thus enhance the performance. In most cases just keep the default value.
> +
> +The available tune data of above events is [0, 1, 2].
> +Writing a negative value will return an error, and out of range
> +values will be converted to 2. Note that the event value just
> +indicates a probable level, but is not precise.

This is useful documentation but it also should be found in the ABI
documentation referred to above.

> +
> +Trace
> +=====
> +
> +PTT trace is designed for dumping the TLP headers to the memory, which
> +can be used to analyze the transactions and usage condition of the PCIe
> +Link. You can choose to filter the traced headers by either requester ID,
> +or those downstream of a set of Root Ports on the same core of the PTT
> +device. It's also supported to trace the headers of certain type and of
> +certain direction.
> +
> +You can use the perf command `perf record` to set the parameters, start
> +trace and get the data. It's also supported to decode the trace
> +data with `perf report`. The control parameters for trace is inputted
> +as event code for each events, which will be further illustrated later.
> +An example usage is like
> +::
> +
> +    $ perf record -e hisi_ptt0_2/filter=0x80001,type=1,direction=1,
> +      format=1/ -- sleep 5
> +
> +This will trace the TLP headers downstream root port 0000:00:10.1 (event
> +code for event 'filter' is 0x80001) with type of posted TLP requests,
> +direction of inbound and traced data format of 8DW.
> +
> +1. filter
> +---------
> +
> +The TLP headers to trace can be filtered by the Root Ports or the requester
> +ID of the endpoints, which are located on the same core of the PTT device.
> +You can set the filter by specifying the `filter` parameter which is required
> +to start the trace. The parameter value is 20 bit. The supported filters and
> +related values are outputted through `available_root_port_filters` and
> +`available_requester_filters` sysfs attributes for Root Ports and Requesters
> +respectively.
> +::
> +
> +    $ cat available_root_port_filters
> +    0000:00:10.0	0x80001
> +    0000:00:11.0	0x80004
> +    $ cat available_requester_filters
> +    0000:01:00.0	0x00100
> +    0000:01:00.1	0x00101

If I remember correctly, one of the rule for sysfs is one line per entry.

> +
> +Note that multiple Root Ports can be specified at one time, but only
> +one Endpoint function can be specified in one trace. Specifying both
> +Root Port and function at the same time is not supported.
> +
> +If no filter is available, reading the related filter sysfs attribute
> +will get an empty string.
> +::
> +
> +    $ cat available_root_port_filters
> +
> +    $ cat available_requester_filters

Those too look overly long, and where to find them is not documented.  As such
users have to guest that it must be somewhere under
/sys/devices/hisi_ptt<sicl_id>_<core_id>/.

More comments tomorrow.

Thanks,
Mathieu

> +
> +Currently the available filters are detected in driver's probe. If the supported
> +devices are removed/added after probe, you may need to reload the driver to update
> +the available filters.
> +
> +2. type
> +-------
> +
> +You can trace the TLP headers of certain types by specifying the `type`
> +parameter, which is required to start the trace. The parameter value is
> +8 bit. Current supported types and related values are shown below:
> +
> +- 8'b00000001: posted requests (P)
> +- 8'b00000010: non-posted requests (NP)
> +- 8'b00000100: completions (CPL)
> +
> +You can specify multiple types when tracing inbound TLP headers, but can only
> +specify one when tracing outbound TLP headers.
> +
> +3. direction
> +------------
> +
> +You can trace the TLP headers from certain direction, which is relative
> +to the Root Port or the PCIe core, by specifying the `direction` parameter.
> +This is optional and the default parameter is inbound. The parameter value
> +is 4 bit. When the desired format is 4DW, directions and related values
> +supported are shown below:
> +
> +- 4'b0000: inbound TLPs (P, NP, CPL)
> +- 4'b0001: outbound TLPs (P, NP, CPL)
> +- 4'b0010: outbound TLPs (P, NP, CPL) and inbound TLPs (P, NP, CPL B)
> +- 4'b0011: outbound TLPs (P, NP, CPL) and inbound TLPs (CPL A)
> +
> +When the desired format is 8DW, directions and related values supported are
> +shown below:
> +
> +- 4'b0000: reserved
> +- 4'b0001: outbound TLPs (P, NP, CPL)
> +- 4'b0010: inbound TLPs (P, NP, CPL B)
> +- 4'b0011: inbound TLPs (CPL A)
> +
> +Inbound completions are classified into two types:
> +
> +- completion A (CPL A): completion of CHI/DMA/Native non-posted requests, except for CPL B
> +- completion B (CPL B): completion of DMA remote2local and P2P non-posted requests
> +
> +4. format
> +--------------
> +
> +You can change the format of the traced TLP headers by specifying the
> +`format` parameter. The default format is 4DW. The parameter value is 4 bit.
> +Current supported formats and related values are shown below:
> +
> +- 4'b0000: 4DW length per TLP header
> +- 4'b0001: 8DW length per TLP header
> +
> +The traced TLP header format is different from the PCIe standard.
> +
> +When using the 8DW data format, the entire TLP header is logged
> +(Header DW0-3 shown below). For example, the TLP header for Memory
> +Reads with 64-bit addresses is shown in PCIe r5.0, Figure 2-17;
> +the header for Configuration Requests is shown in Figure 2.20, etc.
> +
> +In addition, 8DW trace buffer entries contain a timestamp and
> +possibly a prefix for a PASID TLP prefix (see Figure 6-20, PCIe r5.0).
> +Otherwise this field will be all 0.
> +
> +The bit[31:11] of DW0 is always 0x1fffff, which can be
> +used to distinguish the data format. 8DW format is like
> +::
> +
> +    bits [                 31:11                 ][       10:0       ]
> +         |---------------------------------------|-------------------|
> +     DW0 [                0x1fffff               ][ Reserved (0x7ff) ]
> +     DW1 [                       Prefix                              ]
> +     DW2 [                     Header DW0                            ]
> +     DW3 [                     Header DW1                            ]
> +     DW4 [                     Header DW2                            ]
> +     DW5 [                     Header DW3                            ]
> +     DW6 [                   Reserved (0x0)                          ]
> +     DW7 [                        Time                               ]
> +
> +When using the 4DW data format, DW0 of the trace buffer entry
> +contains selected fields of DW0 of the TLP, together with a
> +timestamp.  DW1-DW3 of the trace buffer entry contain DW1-DW3
> +directly from the TLP header.
> +
> +4DW format is like
> +::
> +
> +    bits [31:30] [ 29:25 ][24][23][22][21][    20:11   ][    10:0    ]
> +         |-----|---------|---|---|---|---|-------------|-------------|
> +     DW0 [ Fmt ][  Type  ][T9][T8][TH][SO][   Length   ][    Time    ]
> +     DW1 [                     Header DW1                            ]
> +     DW2 [                     Header DW2                            ]
> +     DW3 [                     Header DW3                            ]
> +
> +5. memory management
> +--------------------
> +
> +The traced TLP headers will be written to the memory allocated
> +by the driver. The hardware accepts 4 DMA address with same size,
> +and writes the buffer sequentially like below. If DMA addr 3 is
> +finished and the trace is still on, it will return to addr 0.
> +::
> +
> +    +->[DMA addr 0]->[DMA addr 1]->[DMA addr 2]->[DMA addr 3]-+
> +    +---------------------------------------------------------+
> +
> +Driver will allocate each DMA buffer of 4MiB. The finished buffer
> +will be copied to the perf AUX buffer allocated by the perf core.
> +Once the AUX buffer is full while the trace is still on, driver
> +will commit the AUX buffer first and then apply for a new one with
> +the same size. The size of AUX buffer is default to 16MiB. User can
> +adjust the size by specifying the `-m` parameter of the perf command.
> +
> +6. decoding
> +-----------
> +
> +You can decode the traced data with `perf report -D` command (currently
> +only support to dump the raw trace data). The traced data will be decoded
> +according to the format described previously (take 8DW as an example):
> +::
> +
> +    [...perf headers and other information]
> +    . ... HISI PTT data: size 4194304 bytes
> +    .  00000000: 00 00 00 00                                 Prefix
> +    .  00000004: 01 00 00 60                                 Header DW0
> +    .  00000008: 0f 1e 00 01                                 Header DW1
> +    .  0000000c: 04 00 00 00                                 Header DW2
> +    .  00000010: 40 00 81 02                                 Header DW3
> +    .  00000014: 33 c0 04 00                                 Time
> +    .  00000020: 00 00 00 00                                 Prefix
> +    .  00000024: 01 00 00 60                                 Header DW0
> +    .  00000028: 0f 1e 00 01                                 Header DW1
> +    .  0000002c: 04 00 00 00                                 Header DW2
> +    .  00000030: 40 00 81 02                                 Header DW3
> +    .  00000034: 02 00 00 00                                 Time
> +    .  00000040: 00 00 00 00                                 Prefix
> +    .  00000044: 01 00 00 60                                 Header DW0
> +    .  00000048: 0f 1e 00 01                                 Header DW1
> +    .  0000004c: 04 00 00 00                                 Header DW2
> +    .  00000050: 40 00 81 02                                 Header DW3
> +    [...]
> diff --git a/Documentation/trace/index.rst b/Documentation/trace/index.rst
> index f9b7bcb5a630..dd423113b7b1 100644
> --- a/Documentation/trace/index.rst
> +++ b/Documentation/trace/index.rst
> @@ -32,3 +32,4 @@ Linux Tracing Technologies
>     sys-t
>     coresight/index
>     user_events
> +   hisi-ptt
> -- 
> 2.24.0
>

On 2022/7/7 1:57, Mathieu Poirier wrote:
> Hi,
> 
> I have started looking at this set.

Thanks!

> 
> On Mon, Jun 06, 2022 at 07:55:54PM +0800, Yicong Yang wrote:
>> Document the introduction and usage of HiSilicon PTT device driver.
>>
>> Signed-off-by: Yicong Yang <yangyicong@hisilicon.com>
>> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
>> ---
>>  Documentation/trace/hisi-ptt.rst | 307 +++++++++++++++++++++++++++++++
>>  Documentation/trace/index.rst    |   1 +
> 
> The "get_maintainer" script clearly indicates that Jonathan Corbet maintains the
> Documentation directory and yet he is not CC'ed on this patch, nor is the
> linux-doc mainling list.  As such, it would not be possible to merge this
> patchset.
> 

sorry for missing. +cc'ed.

>>  2 files changed, 308 insertions(+)
>>  create mode 100644 Documentation/trace/hisi-ptt.rst
>>
>> diff --git a/Documentation/trace/hisi-ptt.rst b/Documentation/trace/hisi-ptt.rst
>> new file mode 100644
>> index 000000000000..0a3112244d40
>> --- /dev/null
>> +++ b/Documentation/trace/hisi-ptt.rst
>> @@ -0,0 +1,307 @@
>> +.. SPDX-License-Identifier: GPL-2.0
>> +
>> +======================================
>> +HiSilicon PCIe Tune and Trace device
>> +======================================
>> +
>> +Introduction
>> +============
>> +
>> +HiSilicon PCIe tune and trace device (PTT) is a PCIe Root Complex
>> +integrated Endpoint (RCiEP) device, providing the capability
>> +to dynamically monitor and tune the PCIe link's events (tune),
>> +and trace the TLP headers (trace). The two functions are independent,
>> +but is recommended to use them together to analyze and enhance the
>> +PCIe link's performance.
>> +
>> +On Kunpeng 930 SoC, the PCIe Root Complex is composed of several
>> +PCIe cores. Each PCIe core includes several Root Ports and a PTT
>> +RCiEP, like below. The PTT device is capable of tuning and
>> +tracing the links of the PCIe core.
>> +::
>> +
>> +          +--------------Core 0-------+
>> +          |       |       [   PTT   ] |
>> +          |       |       [Root Port]---[Endpoint]
>> +          |       |       [Root Port]---[Endpoint]
>> +          |       |       [Root Port]---[Endpoint]
>> +    Root Complex  |------Core 1-------+
>> +          |       |       [   PTT   ] |
>> +          |       |       [Root Port]---[ Switch ]---[Endpoint]
>> +          |       |       [Root Port]---[Endpoint] `-[Endpoint]
>> +          |       |       [Root Port]---[Endpoint]
>> +          +---------------------------+
>> +
>> +The PTT device driver registers one PMU device for each PTT device.
>> +The name of each PTT device is composed of 'hisi_ptt' prefix with
>> +the id of the SICL and the Core where it locates. The Kunpeng 930
>> +SoC encapsulates multiple CPU dies (SCCL, Super CPU Cluster) and
>> +IO dies (SICL, Super I/O Cluster), where there's one PCIe Root
>> +Complex for each SICL.
>> +::
>> +
>> +    /sys/devices/hisi_ptt<sicl_id>_<core_id>
> 
> All entries added to sysfs should have corresponding documentation.  See [1] and
> [2] for details and [3] for an example.
> 
> [1]. https://elixir.bootlin.com/linux/latest/source/Documentation/ABI/README
> [2]. https://elixir.bootlin.com/linux/latest/source/Documentation/ABI/testing
> [3]. https://elixir.bootlin.com/linux/latest/source/Documentation/ABI/testing/sysfs-bus-coresight-devices-etm4x
> 

ok. I'll add a patch for ABI description. Thanks for the reference.

>> +
>> +Tune
>> +====
>> +
>> +PTT tune is designed for monitoring and adjusting PCIe link parameters (events).
>> +Currently we support events in 4 classes. The scope of the events
>> +covers the PCIe core to which the PTT device belongs.
>> +
>> +Each event is presented as a file under $(PTT PMU dir)/tune, and
>> +a simple open/read/write/close cycle will be used to tune the event.
>> +::
>> +
>> +    $ cd /sys/devices/hisi_ptt<sicl_id>_<core_id>/tune
>> +    $ ls
>> +    qos_tx_cpl    qos_tx_np    qos_tx_p
>> +    tx_path_rx_req_alloc_buf_level
>> +    tx_path_tx_req_alloc_buf_level
> 
> These look overly long... How about watermark_rx and watermark_tx?
> 

These are gotten from the hardware manual and abbreviated. These events are highly connected
to the hardware desgin so I think it's better to keep consistence. The watermark_{rx, tx} will
become ambigious when we add more events for Rx path or other Tx path events.

The event code is composed of two parts. First part (tx_path) describes which path it belongs to
and second part describes the function ({rx,tx}_req_alloc_buf_level). We called the link path
between CPU and PCIe RC as Rx path and the path between PCIe RC to the PCIe link as Tx path.
So we need to have tx_path prefix for the Tx path and {rx, tx}_req_alloc_buf_level for the
requested watermark of {inbound, outbound} buffer allocation. Indeed we have other Tx path
buffer events which are not exported in this series.


>> +    $ cat qos_tx_dp
>> +    1
>> +    $ echo 2 > qos_tx_dp
>> +    $ cat qos_tx_dp
>> +    2
>> +
>> +Current value (numerical value) of the event can be simply read
>> +from the file, and the desired value written to the file to tune.
>> +
>> +1. Tx path QoS control
>> +------------------------
>> +
>> +The following files are provided to tune the QoS of the tx path of
>> +the PCIe core.
>> +
>> +- qos_tx_cpl: weight of Tx completion TLPs
>> +- qos_tx_np: weight of Tx non-posted TLPs
>> +- qos_tx_p: weight of Tx posted TLPs
>> +
>> +The weight influences the proportion of certain packets on the PCIe link.
>> +For example, for the storage scenario, increase the proportion
>> +of the completion packets on the link to enhance the performance as
>> +more completions are consumed.
>> +
>> +The available tune data of these events is [0, 1, 2].
>> +Writing a negative value will return an error, and out of range
>> +values will be converted to 2. Note that the event value just
>> +indicates a probable level, but is not precise.
>> +
>> +2. Tx path buffer control
>> +-------------------------
>> +
>> +Following files are provided to tune the buffer of tx path of the PCIe core.
>> +
>> +- tx_path_rx_req_alloc_buf_level: watermark of Rx requested
>> +- tx_path_tx_req_alloc_buf_level: watermark of Tx requested
>> +
>> +These events influence the watermark of the buffer allocated for each
>> +type. Rx means the inbound while Tx means outbound. The packets will
>> +be stored in the buffer first and then transmitted either when the
>> +watermark reached or when timed out. For a busy direction, you should
>> +increase the related buffer watermark to avoid frequently posting and
>> +thus enhance the performance. In most cases just keep the default value.
>> +
>> +The available tune data of above events is [0, 1, 2].
>> +Writing a negative value will return an error, and out of range
>> +values will be converted to 2. Note that the event value just
>> +indicates a probable level, but is not precise.
> 
> This is useful documentation but it also should be found in the ABI
> documentation referred to above.
> 
>> +
>> +Trace
>> +=====
>> +
>> +PTT trace is designed for dumping the TLP headers to the memory, which
>> +can be used to analyze the transactions and usage condition of the PCIe
>> +Link. You can choose to filter the traced headers by either requester ID,
>> +or those downstream of a set of Root Ports on the same core of the PTT
>> +device. It's also supported to trace the headers of certain type and of
>> +certain direction.
>> +
>> +You can use the perf command `perf record` to set the parameters, start
>> +trace and get the data. It's also supported to decode the trace
>> +data with `perf report`. The control parameters for trace is inputted
>> +as event code for each events, which will be further illustrated later.
>> +An example usage is like
>> +::
>> +
>> +    $ perf record -e hisi_ptt0_2/filter=0x80001,type=1,direction=1,
>> +      format=1/ -- sleep 5
>> +
>> +This will trace the TLP headers downstream root port 0000:00:10.1 (event
>> +code for event 'filter' is 0x80001) with type of posted TLP requests,
>> +direction of inbound and traced data format of 8DW.
>> +
>> +1. filter
>> +---------
>> +
>> +The TLP headers to trace can be filtered by the Root Ports or the requester
>> +ID of the endpoints, which are located on the same core of the PTT device.
>> +You can set the filter by specifying the `filter` parameter which is required
>> +to start the trace. The parameter value is 20 bit. The supported filters and
>> +related values are outputted through `available_root_port_filters` and
>> +`available_requester_filters` sysfs attributes for Root Ports and Requesters
>> +respectively.
>> +::
>> +
>> +    $ cat available_root_port_filters
>> +    0000:00:10.0	0x80001
>> +    0000:00:11.0	0x80004
>> +    $ cat available_requester_filters
>> +    0000:01:00.0	0x00100
>> +    0000:01:00.1	0x00101
> 
> If I remember correctly, one of the rule for sysfs is one line per entry.
> 

Since one PTT devices may support several Root Ports and Endpoints on its core, I find no better
way to make this information convenient and easy to use for the users to collect. So maybe this
canbe an exception and there seems to have some limited examples like
/sys/devices/system/node/node<N>/{meminfo, vmstat, meminfo}.

>> +
>> +Note that multiple Root Ports can be specified at one time, but only
>> +one Endpoint function can be specified in one trace. Specifying both
>> +Root Port and function at the same time is not supported.
>> +
>> +If no filter is available, reading the related filter sysfs attribute
>> +will get an empty string.
>> +::
>> +
>> +    $ cat available_root_port_filters
>> +
>> +    $ cat available_requester_filters
> 
> Those too look overly long, and where to find them is not documented.  As such
> users have to guest that it must be somewhere under
> /sys/devices/hisi_ptt<sicl_id>_<core_id>/.
> 

Since Root Port and Requester are PCIe terminologies so it's better to have them
embedded to make it clear. Maybe 'available' can be removed.

Will have all these sysfs attributes documented.

> More comments tomorrow.
> 

Thanks,
Yicong

On Thu, Jul 07, 2022 at 07:43:21PM +0800, Yicong Yang wrote:
> On 2022/7/7 1:57, Mathieu Poirier wrote:
> > Hi,
> > 
> > I have started looking at this set.
> 
> Thanks!
> 
> > 
> > On Mon, Jun 06, 2022 at 07:55:54PM +0800, Yicong Yang wrote:
> >> Document the introduction and usage of HiSilicon PTT device driver.
> >>
> >> Signed-off-by: Yicong Yang <yangyicong@hisilicon.com>
> >> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> >> ---
> >>  Documentation/trace/hisi-ptt.rst | 307 +++++++++++++++++++++++++++++++
> >>  Documentation/trace/index.rst    |   1 +
> > 
> > The "get_maintainer" script clearly indicates that Jonathan Corbet maintains the
> > Documentation directory and yet he is not CC'ed on this patch, nor is the
> > linux-doc mainling list.  As such, it would not be possible to merge this
> > patchset.
> > 
> 
> sorry for missing. +cc'ed.
> 
> >>  2 files changed, 308 insertions(+)
> >>  create mode 100644 Documentation/trace/hisi-ptt.rst
> >>
> >> diff --git a/Documentation/trace/hisi-ptt.rst b/Documentation/trace/hisi-ptt.rst
> >> new file mode 100644
> >> index 000000000000..0a3112244d40
> >> --- /dev/null
> >> +++ b/Documentation/trace/hisi-ptt.rst
> >> @@ -0,0 +1,307 @@
> >> +.. SPDX-License-Identifier: GPL-2.0
> >> +
> >> +======================================
> >> +HiSilicon PCIe Tune and Trace device
> >> +======================================
> >> +
> >> +Introduction
> >> +============
> >> +
> >> +HiSilicon PCIe tune and trace device (PTT) is a PCIe Root Complex
> >> +integrated Endpoint (RCiEP) device, providing the capability
> >> +to dynamically monitor and tune the PCIe link's events (tune),
> >> +and trace the TLP headers (trace). The two functions are independent,
> >> +but is recommended to use them together to analyze and enhance the
> >> +PCIe link's performance.
> >> +
> >> +On Kunpeng 930 SoC, the PCIe Root Complex is composed of several
> >> +PCIe cores. Each PCIe core includes several Root Ports and a PTT
> >> +RCiEP, like below. The PTT device is capable of tuning and
> >> +tracing the links of the PCIe core.
> >> +::
> >> +
> >> +          +--------------Core 0-------+
> >> +          |       |       [   PTT   ] |
> >> +          |       |       [Root Port]---[Endpoint]
> >> +          |       |       [Root Port]---[Endpoint]
> >> +          |       |       [Root Port]---[Endpoint]
> >> +    Root Complex  |------Core 1-------+
> >> +          |       |       [   PTT   ] |
> >> +          |       |       [Root Port]---[ Switch ]---[Endpoint]
> >> +          |       |       [Root Port]---[Endpoint] `-[Endpoint]
> >> +          |       |       [Root Port]---[Endpoint]
> >> +          +---------------------------+
> >> +
> >> +The PTT device driver registers one PMU device for each PTT device.
> >> +The name of each PTT device is composed of 'hisi_ptt' prefix with
> >> +the id of the SICL and the Core where it locates. The Kunpeng 930
> >> +SoC encapsulates multiple CPU dies (SCCL, Super CPU Cluster) and
> >> +IO dies (SICL, Super I/O Cluster), where there's one PCIe Root
> >> +Complex for each SICL.
> >> +::
> >> +
> >> +    /sys/devices/hisi_ptt<sicl_id>_<core_id>
> > 
> > All entries added to sysfs should have corresponding documentation.  See [1] and
> > [2] for details and [3] for an example.
> > 
> > [1]. https://elixir.bootlin.com/linux/latest/source/Documentation/ABI/README
> > [2]. https://elixir.bootlin.com/linux/latest/source/Documentation/ABI/testing
> > [3]. https://elixir.bootlin.com/linux/latest/source/Documentation/ABI/testing/sysfs-bus-coresight-devices-etm4x
> > 
> 
> ok. I'll add a patch for ABI description. Thanks for the reference.
> 
> >> +
> >> +Tune
> >> +====
> >> +
> >> +PTT tune is designed for monitoring and adjusting PCIe link parameters (events).
> >> +Currently we support events in 4 classes. The scope of the events
> >> +covers the PCIe core to which the PTT device belongs.
> >> +
> >> +Each event is presented as a file under $(PTT PMU dir)/tune, and
> >> +a simple open/read/write/close cycle will be used to tune the event.
> >> +::
> >> +
> >> +    $ cd /sys/devices/hisi_ptt<sicl_id>_<core_id>/tune
> >> +    $ ls
> >> +    qos_tx_cpl    qos_tx_np    qos_tx_p
> >> +    tx_path_rx_req_alloc_buf_level
> >> +    tx_path_tx_req_alloc_buf_level
> > 
> > These look overly long... How about watermark_rx and watermark_tx?
> > 
> 
> These are gotten from the hardware manual and abbreviated. These events are highly connected
> to the hardware desgin so I think it's better to keep consistence. The watermark_{rx, tx} will
> become ambigious when we add more events for Rx path or other Tx path events.
> 
> The event code is composed of two parts. First part (tx_path) describes which path it belongs to
> and second part describes the function ({rx,tx}_req_alloc_buf_level). We called the link path
> between CPU and PCIe RC as Rx path and the path between PCIe RC to the PCIe link as Tx path.
> So we need to have tx_path prefix for the Tx path and {rx, tx}_req_alloc_buf_level for the
> requested watermark of {inbound, outbound} buffer allocation. Indeed we have other Tx path
> buffer events which are not exported in this series.
>

I will not be maintaining nor using this driver so the choice is entirely yours.
That being said the end result is the same - those names are, in my opinion, too
long.

> 
> >> +    $ cat qos_tx_dp
> >> +    1
> >> +    $ echo 2 > qos_tx_dp
> >> +    $ cat qos_tx_dp
> >> +    2
> >> +
> >> +Current value (numerical value) of the event can be simply read
> >> +from the file, and the desired value written to the file to tune.
> >> +
> >> +1. Tx path QoS control
> >> +------------------------
> >> +
> >> +The following files are provided to tune the QoS of the tx path of
> >> +the PCIe core.
> >> +
> >> +- qos_tx_cpl: weight of Tx completion TLPs
> >> +- qos_tx_np: weight of Tx non-posted TLPs
> >> +- qos_tx_p: weight of Tx posted TLPs
> >> +
> >> +The weight influences the proportion of certain packets on the PCIe link.
> >> +For example, for the storage scenario, increase the proportion
> >> +of the completion packets on the link to enhance the performance as
> >> +more completions are consumed.
> >> +
> >> +The available tune data of these events is [0, 1, 2].
> >> +Writing a negative value will return an error, and out of range
> >> +values will be converted to 2. Note that the event value just
> >> +indicates a probable level, but is not precise.
> >> +
> >> +2. Tx path buffer control
> >> +-------------------------
> >> +
> >> +Following files are provided to tune the buffer of tx path of the PCIe core.
> >> +
> >> +- tx_path_rx_req_alloc_buf_level: watermark of Rx requested
> >> +- tx_path_tx_req_alloc_buf_level: watermark of Tx requested
> >> +
> >> +These events influence the watermark of the buffer allocated for each
> >> +type. Rx means the inbound while Tx means outbound. The packets will
> >> +be stored in the buffer first and then transmitted either when the
> >> +watermark reached or when timed out. For a busy direction, you should
> >> +increase the related buffer watermark to avoid frequently posting and
> >> +thus enhance the performance. In most cases just keep the default value.
> >> +
> >> +The available tune data of above events is [0, 1, 2].
> >> +Writing a negative value will return an error, and out of range
> >> +values will be converted to 2. Note that the event value just
> >> +indicates a probable level, but is not precise.
> > 
> > This is useful documentation but it also should be found in the ABI
> > documentation referred to above.
> > 
> >> +
> >> +Trace
> >> +=====
> >> +
> >> +PTT trace is designed for dumping the TLP headers to the memory, which
> >> +can be used to analyze the transactions and usage condition of the PCIe
> >> +Link. You can choose to filter the traced headers by either requester ID,
> >> +or those downstream of a set of Root Ports on the same core of the PTT
> >> +device. It's also supported to trace the headers of certain type and of
> >> +certain direction.
> >> +
> >> +You can use the perf command `perf record` to set the parameters, start
> >> +trace and get the data. It's also supported to decode the trace
> >> +data with `perf report`. The control parameters for trace is inputted
> >> +as event code for each events, which will be further illustrated later.
> >> +An example usage is like
> >> +::
> >> +
> >> +    $ perf record -e hisi_ptt0_2/filter=0x80001,type=1,direction=1,
> >> +      format=1/ -- sleep 5
> >> +
> >> +This will trace the TLP headers downstream root port 0000:00:10.1 (event
> >> +code for event 'filter' is 0x80001) with type of posted TLP requests,
> >> +direction of inbound and traced data format of 8DW.
> >> +
> >> +1. filter
> >> +---------
> >> +
> >> +The TLP headers to trace can be filtered by the Root Ports or the requester
> >> +ID of the endpoints, which are located on the same core of the PTT device.
> >> +You can set the filter by specifying the `filter` parameter which is required
> >> +to start the trace. The parameter value is 20 bit. The supported filters and
> >> +related values are outputted through `available_root_port_filters` and
> >> +`available_requester_filters` sysfs attributes for Root Ports and Requesters
> >> +respectively.
> >> +::
> >> +
> >> +    $ cat available_root_port_filters
> >> +    0000:00:10.0	0x80001
> >> +    0000:00:11.0	0x80004
> >> +    $ cat available_requester_filters
> >> +    0000:01:00.0	0x00100
> >> +    0000:01:00.1	0x00101
> > 
> > If I remember correctly, one of the rule for sysfs is one line per entry.
> > 
> 
> Since one PTT devices may support several Root Ports and Endpoints on its core, I find no better
> way to make this information convenient and easy to use for the users to collect. So maybe this
> canbe an exception and there seems to have some limited examples like
> /sys/devices/system/node/node<N>/{meminfo, vmstat, meminfo}.

You can either find a better solution or argue the matter with Greg.  I suggest
to introduce new directories, i.e "root_port_filters" and "requested_filters"
and under those have entries like "port0", "port1" and so on.

> 
> >> +
> >> +Note that multiple Root Ports can be specified at one time, but only
> >> +one Endpoint function can be specified in one trace. Specifying both
> >> +Root Port and function at the same time is not supported.
> >> +
> >> +If no filter is available, reading the related filter sysfs attribute
> >> +will get an empty string.
> >> +::
> >> +
> >> +    $ cat available_root_port_filters
> >> +
> >> +    $ cat available_requester_filters
> > 
> > Those too look overly long, and where to find them is not documented.  As such
> > users have to guest that it must be somewhere under
> > /sys/devices/hisi_ptt<sicl_id>_<core_id>/.
> > 
> 
> Since Root Port and Requester are PCIe terminologies so it's better to have them
> embedded to make it clear. Maybe 'available' can be removed.
> 
> Will have all these sysfs attributes documented.
> 
> > More comments tomorrow.
> > 
> 
> Thanks,
> Yicong

On 2022/7/8 0:20, Mathieu Poirier wrote:
> On Thu, Jul 07, 2022 at 07:43:21PM +0800, Yicong Yang wrote:
>> On 2022/7/7 1:57, Mathieu Poirier wrote:
>>> Hi,
>>>
>>> I have started looking at this set.
>>
>> Thanks!
>>
>>>
>>> On Mon, Jun 06, 2022 at 07:55:54PM +0800, Yicong Yang wrote:
>>>> Document the introduction and usage of HiSilicon PTT device driver.
>>>>
>>>> Signed-off-by: Yicong Yang <yangyicong@hisilicon.com>
>>>> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
>>>> ---
>>>>  Documentation/trace/hisi-ptt.rst | 307 +++++++++++++++++++++++++++++++
>>>>  Documentation/trace/index.rst    |   1 +
>>>
>>> The "get_maintainer" script clearly indicates that Jonathan Corbet maintains the
>>> Documentation directory and yet he is not CC'ed on this patch, nor is the
>>> linux-doc mainling list.  As such, it would not be possible to merge this
>>> patchset.
>>>
>>
>> sorry for missing. +cc'ed.
>>
>>>>  2 files changed, 308 insertions(+)
>>>>  create mode 100644 Documentation/trace/hisi-ptt.rst
>>>>
>>>> diff --git a/Documentation/trace/hisi-ptt.rst b/Documentation/trace/hisi-ptt.rst
>>>> new file mode 100644
>>>> index 000000000000..0a3112244d40
>>>> --- /dev/null
>>>> +++ b/Documentation/trace/hisi-ptt.rst
>>>> @@ -0,0 +1,307 @@
>>>> +.. SPDX-License-Identifier: GPL-2.0
>>>> +
>>>> +======================================
>>>> +HiSilicon PCIe Tune and Trace device
>>>> +======================================
>>>> +
>>>> +Introduction
>>>> +============
>>>> +
>>>> +HiSilicon PCIe tune and trace device (PTT) is a PCIe Root Complex
>>>> +integrated Endpoint (RCiEP) device, providing the capability
>>>> +to dynamically monitor and tune the PCIe link's events (tune),
>>>> +and trace the TLP headers (trace). The two functions are independent,
>>>> +but is recommended to use them together to analyze and enhance the
>>>> +PCIe link's performance.
>>>> +
>>>> +On Kunpeng 930 SoC, the PCIe Root Complex is composed of several
>>>> +PCIe cores. Each PCIe core includes several Root Ports and a PTT
>>>> +RCiEP, like below. The PTT device is capable of tuning and
>>>> +tracing the links of the PCIe core.
>>>> +::
>>>> +
>>>> +          +--------------Core 0-------+
>>>> +          |       |       [   PTT   ] |
>>>> +          |       |       [Root Port]---[Endpoint]
>>>> +          |       |       [Root Port]---[Endpoint]
>>>> +          |       |       [Root Port]---[Endpoint]
>>>> +    Root Complex  |------Core 1-------+
>>>> +          |       |       [   PTT   ] |
>>>> +          |       |       [Root Port]---[ Switch ]---[Endpoint]
>>>> +          |       |       [Root Port]---[Endpoint] `-[Endpoint]
>>>> +          |       |       [Root Port]---[Endpoint]
>>>> +          +---------------------------+
>>>> +
>>>> +The PTT device driver registers one PMU device for each PTT device.
>>>> +The name of each PTT device is composed of 'hisi_ptt' prefix with
>>>> +the id of the SICL and the Core where it locates. The Kunpeng 930
>>>> +SoC encapsulates multiple CPU dies (SCCL, Super CPU Cluster) and
>>>> +IO dies (SICL, Super I/O Cluster), where there's one PCIe Root
>>>> +Complex for each SICL.
>>>> +::
>>>> +
>>>> +    /sys/devices/hisi_ptt<sicl_id>_<core_id>
>>>
>>> All entries added to sysfs should have corresponding documentation.  See [1] and
>>> [2] for details and [3] for an example.
>>>
>>> [1]. https://elixir.bootlin.com/linux/latest/source/Documentation/ABI/README
>>> [2]. https://elixir.bootlin.com/linux/latest/source/Documentation/ABI/testing
>>> [3]. https://elixir.bootlin.com/linux/latest/source/Documentation/ABI/testing/sysfs-bus-coresight-devices-etm4x
>>>
>>
>> ok. I'll add a patch for ABI description. Thanks for the reference.
>>
>>>> +
>>>> +Tune
>>>> +====
>>>> +
>>>> +PTT tune is designed for monitoring and adjusting PCIe link parameters (events).
>>>> +Currently we support events in 4 classes. The scope of the events
>>>> +covers the PCIe core to which the PTT device belongs.
>>>> +
>>>> +Each event is presented as a file under $(PTT PMU dir)/tune, and
>>>> +a simple open/read/write/close cycle will be used to tune the event.
>>>> +::
>>>> +
>>>> +    $ cd /sys/devices/hisi_ptt<sicl_id>_<core_id>/tune
>>>> +    $ ls
>>>> +    qos_tx_cpl    qos_tx_np    qos_tx_p
>>>> +    tx_path_rx_req_alloc_buf_level
>>>> +    tx_path_tx_req_alloc_buf_level
>>>
>>> These look overly long... How about watermark_rx and watermark_tx?
>>>
>>
>> These are gotten from the hardware manual and abbreviated. These events are highly connected
>> to the hardware desgin so I think it's better to keep consistence. The watermark_{rx, tx} will
>> become ambigious when we add more events for Rx path or other Tx path events.
>>
>> The event code is composed of two parts. First part (tx_path) describes which path it belongs to
>> and second part describes the function ({rx,tx}_req_alloc_buf_level). We called the link path
>> between CPU and PCIe RC as Rx path and the path between PCIe RC to the PCIe link as Tx path.
>> So we need to have tx_path prefix for the Tx path and {rx, tx}_req_alloc_buf_level for the
>> requested watermark of {inbound, outbound} buffer allocation. Indeed we have other Tx path
>> buffer events which are not exported in this series.
>>
> 
> I will not be maintaining nor using this driver so the choice is entirely yours.
> That being said the end result is the same - those names are, in my opinion, too
> long.
> 
>>
>>>> +    $ cat qos_tx_dp
>>>> +    1
>>>> +    $ echo 2 > qos_tx_dp
>>>> +    $ cat qos_tx_dp
>>>> +    2
>>>> +
>>>> +Current value (numerical value) of the event can be simply read
>>>> +from the file, and the desired value written to the file to tune.
>>>> +
>>>> +1. Tx path QoS control
>>>> +------------------------
>>>> +
>>>> +The following files are provided to tune the QoS of the tx path of
>>>> +the PCIe core.
>>>> +
>>>> +- qos_tx_cpl: weight of Tx completion TLPs
>>>> +- qos_tx_np: weight of Tx non-posted TLPs
>>>> +- qos_tx_p: weight of Tx posted TLPs
>>>> +
>>>> +The weight influences the proportion of certain packets on the PCIe link.
>>>> +For example, for the storage scenario, increase the proportion
>>>> +of the completion packets on the link to enhance the performance as
>>>> +more completions are consumed.
>>>> +
>>>> +The available tune data of these events is [0, 1, 2].
>>>> +Writing a negative value will return an error, and out of range
>>>> +values will be converted to 2. Note that the event value just
>>>> +indicates a probable level, but is not precise.
>>>> +
>>>> +2. Tx path buffer control
>>>> +-------------------------
>>>> +
>>>> +Following files are provided to tune the buffer of tx path of the PCIe core.
>>>> +
>>>> +- tx_path_rx_req_alloc_buf_level: watermark of Rx requested
>>>> +- tx_path_tx_req_alloc_buf_level: watermark of Tx requested
>>>> +
>>>> +These events influence the watermark of the buffer allocated for each
>>>> +type. Rx means the inbound while Tx means outbound. The packets will
>>>> +be stored in the buffer first and then transmitted either when the
>>>> +watermark reached or when timed out. For a busy direction, you should
>>>> +increase the related buffer watermark to avoid frequently posting and
>>>> +thus enhance the performance. In most cases just keep the default value.
>>>> +
>>>> +The available tune data of above events is [0, 1, 2].
>>>> +Writing a negative value will return an error, and out of range
>>>> +values will be converted to 2. Note that the event value just
>>>> +indicates a probable level, but is not precise.
>>>
>>> This is useful documentation but it also should be found in the ABI
>>> documentation referred to above.
>>>
>>>> +
>>>> +Trace
>>>> +=====
>>>> +
>>>> +PTT trace is designed for dumping the TLP headers to the memory, which
>>>> +can be used to analyze the transactions and usage condition of the PCIe
>>>> +Link. You can choose to filter the traced headers by either requester ID,
>>>> +or those downstream of a set of Root Ports on the same core of the PTT
>>>> +device. It's also supported to trace the headers of certain type and of
>>>> +certain direction.
>>>> +
>>>> +You can use the perf command `perf record` to set the parameters, start
>>>> +trace and get the data. It's also supported to decode the trace
>>>> +data with `perf report`. The control parameters for trace is inputted
>>>> +as event code for each events, which will be further illustrated later.
>>>> +An example usage is like
>>>> +::
>>>> +
>>>> +    $ perf record -e hisi_ptt0_2/filter=0x80001,type=1,direction=1,
>>>> +      format=1/ -- sleep 5
>>>> +
>>>> +This will trace the TLP headers downstream root port 0000:00:10.1 (event
>>>> +code for event 'filter' is 0x80001) with type of posted TLP requests,
>>>> +direction of inbound and traced data format of 8DW.
>>>> +
>>>> +1. filter
>>>> +---------
>>>> +
>>>> +The TLP headers to trace can be filtered by the Root Ports or the requester
>>>> +ID of the endpoints, which are located on the same core of the PTT device.
>>>> +You can set the filter by specifying the `filter` parameter which is required
>>>> +to start the trace. The parameter value is 20 bit. The supported filters and
>>>> +related values are outputted through `available_root_port_filters` and
>>>> +`available_requester_filters` sysfs attributes for Root Ports and Requesters
>>>> +respectively.
>>>> +::
>>>> +
>>>> +    $ cat available_root_port_filters
>>>> +    0000:00:10.0	0x80001
>>>> +    0000:00:11.0	0x80004
>>>> +    $ cat available_requester_filters
>>>> +    0000:01:00.0	0x00100
>>>> +    0000:01:00.1	0x00101
>>>
>>> If I remember correctly, one of the rule for sysfs is one line per entry.
>>>
>>
>> Since one PTT devices may support several Root Ports and Endpoints on its core, I find no better
>> way to make this information convenient and easy to use for the users to collect. So maybe this
>> canbe an exception and there seems to have some limited examples like
>> /sys/devices/system/node/node<N>/{meminfo, vmstat, meminfo}.
> 
> You can either find a better solution or argue the matter with Greg.  I suggest
> to introduce new directories, i.e "root_port_filters" and "requested_filters"
> and under those have entries like "port0", "port1" and so on.
> 

Thanks for the suggestion and it does make sense to me. But I've tried it and met some problems.
I intended to create two attribute groups for each kind of filter and create attribute for each
available filter. like:

$ ls root_port_filters    # each filter exists as an sysfs attribute
0000:00:10.0 0000:00:11.0
$ cat root_port_filters/0000:00:10.0    # read the attribute get the filter value
0x80001

In pratice we may have no filter for certain types, for example no endpoint found on this PCIe
core then we'll have no filter attributes under requester_filters directory. Or if endpoint
devices are hot removed or manually removed through sysfs by the user, the filter attributes
will be created/destroyed dynamically (will support following this series, an implementation
will be like in [1]) which may make the directory empty. But sysfs won't allow to have an
empty attribute group, restricted in [2].

[1] https://lore.kernel.org/lkml/20220407125841.3678-4-yangyicong@hisilicon.com/
[2] https://github.com/torvalds/linux/blob/5cf3bb0d3a2d0de94f3f551f0e4211068818aabf/fs/sysfs/group.c#L121

>>
>>>> +
>>>> +Note that multiple Root Ports can be specified at one time, but only
>>>> +one Endpoint function can be specified in one trace. Specifying both
>>>> +Root Port and function at the same time is not supported.
>>>> +
>>>> +If no filter is available, reading the related filter sysfs attribute
>>>> +will get an empty string.
>>>> +::
>>>> +
>>>> +    $ cat available_root_port_filters
>>>> +
>>>> +    $ cat available_requester_filters
>>>
>>> Those too look overly long, and where to find them is not documented.  As such
>>> users have to guest that it must be somewhere under
>>> /sys/devices/hisi_ptt<sicl_id>_<core_id>/.
>>>
>>
>> Since Root Port and Requester are PCIe terminologies so it's better to have them
>> embedded to make it clear. Maybe 'available' can be removed.
>>
>> Will have all these sysfs attributes documented.
>>
>>> More comments tomorrow.
>>>
>>
>> Thanks,
>> Yicong
> .
>

On Tue, 12 Jul 2022 at 06:38, Yicong Yang <yangyicong@huawei.com> wrote:
>
> On 2022/7/8 0:20, Mathieu Poirier wrote:
> > On Thu, Jul 07, 2022 at 07:43:21PM +0800, Yicong Yang wrote:
> >> On 2022/7/7 1:57, Mathieu Poirier wrote:
> >>> Hi,
> >>>
> >>> I have started looking at this set.
> >>
> >> Thanks!
> >>
> >>>
> >>> On Mon, Jun 06, 2022 at 07:55:54PM +0800, Yicong Yang wrote:
> >>>> Document the introduction and usage of HiSilicon PTT device driver.
> >>>>
> >>>> Signed-off-by: Yicong Yang <yangyicong@hisilicon.com>
> >>>> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> >>>> ---
> >>>>  Documentation/trace/hisi-ptt.rst | 307 +++++++++++++++++++++++++++++++
> >>>>  Documentation/trace/index.rst    |   1 +
> >>>
> >>> The "get_maintainer" script clearly indicates that Jonathan Corbet maintains the
> >>> Documentation directory and yet he is not CC'ed on this patch, nor is the
> >>> linux-doc mainling list.  As such, it would not be possible to merge this
> >>> patchset.
> >>>
> >>
> >> sorry for missing. +cc'ed.
> >>
> >>>>  2 files changed, 308 insertions(+)
> >>>>  create mode 100644 Documentation/trace/hisi-ptt.rst
> >>>>
> >>>> diff --git a/Documentation/trace/hisi-ptt.rst b/Documentation/trace/hisi-ptt.rst
> >>>> new file mode 100644
> >>>> index 000000000000..0a3112244d40
> >>>> --- /dev/null
> >>>> +++ b/Documentation/trace/hisi-ptt.rst
> >>>> @@ -0,0 +1,307 @@
> >>>> +.. SPDX-License-Identifier: GPL-2.0
> >>>> +
> >>>> +======================================
> >>>> +HiSilicon PCIe Tune and Trace device
> >>>> +======================================
> >>>> +
> >>>> +Introduction
> >>>> +============
> >>>> +
> >>>> +HiSilicon PCIe tune and trace device (PTT) is a PCIe Root Complex
> >>>> +integrated Endpoint (RCiEP) device, providing the capability
> >>>> +to dynamically monitor and tune the PCIe link's events (tune),
> >>>> +and trace the TLP headers (trace). The two functions are independent,
> >>>> +but is recommended to use them together to analyze and enhance the
> >>>> +PCIe link's performance.
> >>>> +
> >>>> +On Kunpeng 930 SoC, the PCIe Root Complex is composed of several
> >>>> +PCIe cores. Each PCIe core includes several Root Ports and a PTT
> >>>> +RCiEP, like below. The PTT device is capable of tuning and
> >>>> +tracing the links of the PCIe core.
> >>>> +::
> >>>> +
> >>>> +          +--------------Core 0-------+
> >>>> +          |       |       [   PTT   ] |
> >>>> +          |       |       [Root Port]---[Endpoint]
> >>>> +          |       |       [Root Port]---[Endpoint]
> >>>> +          |       |       [Root Port]---[Endpoint]
> >>>> +    Root Complex  |------Core 1-------+
> >>>> +          |       |       [   PTT   ] |
> >>>> +          |       |       [Root Port]---[ Switch ]---[Endpoint]
> >>>> +          |       |       [Root Port]---[Endpoint] `-[Endpoint]
> >>>> +          |       |       [Root Port]---[Endpoint]
> >>>> +          +---------------------------+
> >>>> +
> >>>> +The PTT device driver registers one PMU device for each PTT device.
> >>>> +The name of each PTT device is composed of 'hisi_ptt' prefix with
> >>>> +the id of the SICL and the Core where it locates. The Kunpeng 930
> >>>> +SoC encapsulates multiple CPU dies (SCCL, Super CPU Cluster) and
> >>>> +IO dies (SICL, Super I/O Cluster), where there's one PCIe Root
> >>>> +Complex for each SICL.
> >>>> +::
> >>>> +
> >>>> +    /sys/devices/hisi_ptt<sicl_id>_<core_id>
> >>>
> >>> All entries added to sysfs should have corresponding documentation.  See [1] and
> >>> [2] for details and [3] for an example.
> >>>
> >>> [1]. https://elixir.bootlin.com/linux/latest/source/Documentation/ABI/README
> >>> [2]. https://elixir.bootlin.com/linux/latest/source/Documentation/ABI/testing
> >>> [3]. https://elixir.bootlin.com/linux/latest/source/Documentation/ABI/testing/sysfs-bus-coresight-devices-etm4x
> >>>
> >>
> >> ok. I'll add a patch for ABI description. Thanks for the reference.
> >>
> >>>> +
> >>>> +Tune
> >>>> +====
> >>>> +
> >>>> +PTT tune is designed for monitoring and adjusting PCIe link parameters (events).
> >>>> +Currently we support events in 4 classes. The scope of the events
> >>>> +covers the PCIe core to which the PTT device belongs.
> >>>> +
> >>>> +Each event is presented as a file under $(PTT PMU dir)/tune, and
> >>>> +a simple open/read/write/close cycle will be used to tune the event.
> >>>> +::
> >>>> +
> >>>> +    $ cd /sys/devices/hisi_ptt<sicl_id>_<core_id>/tune
> >>>> +    $ ls
> >>>> +    qos_tx_cpl    qos_tx_np    qos_tx_p
> >>>> +    tx_path_rx_req_alloc_buf_level
> >>>> +    tx_path_tx_req_alloc_buf_level
> >>>
> >>> These look overly long... How about watermark_rx and watermark_tx?
> >>>
> >>
> >> These are gotten from the hardware manual and abbreviated. These events are highly connected
> >> to the hardware desgin so I think it's better to keep consistence. The watermark_{rx, tx} will
> >> become ambigious when we add more events for Rx path or other Tx path events.
> >>
> >> The event code is composed of two parts. First part (tx_path) describes which path it belongs to
> >> and second part describes the function ({rx,tx}_req_alloc_buf_level). We called the link path
> >> between CPU and PCIe RC as Rx path and the path between PCIe RC to the PCIe link as Tx path.
> >> So we need to have tx_path prefix for the Tx path and {rx, tx}_req_alloc_buf_level for the
> >> requested watermark of {inbound, outbound} buffer allocation. Indeed we have other Tx path
> >> buffer events which are not exported in this series.
> >>
> >
> > I will not be maintaining nor using this driver so the choice is entirely yours.
> > That being said the end result is the same - those names are, in my opinion, too
> > long.
> >
> >>
> >>>> +    $ cat qos_tx_dp
> >>>> +    1
> >>>> +    $ echo 2 > qos_tx_dp
> >>>> +    $ cat qos_tx_dp
> >>>> +    2
> >>>> +
> >>>> +Current value (numerical value) of the event can be simply read
> >>>> +from the file, and the desired value written to the file to tune.
> >>>> +
> >>>> +1. Tx path QoS control
> >>>> +------------------------
> >>>> +
> >>>> +The following files are provided to tune the QoS of the tx path of
> >>>> +the PCIe core.
> >>>> +
> >>>> +- qos_tx_cpl: weight of Tx completion TLPs
> >>>> +- qos_tx_np: weight of Tx non-posted TLPs
> >>>> +- qos_tx_p: weight of Tx posted TLPs
> >>>> +
> >>>> +The weight influences the proportion of certain packets on the PCIe link.
> >>>> +For example, for the storage scenario, increase the proportion
> >>>> +of the completion packets on the link to enhance the performance as
> >>>> +more completions are consumed.
> >>>> +
> >>>> +The available tune data of these events is [0, 1, 2].
> >>>> +Writing a negative value will return an error, and out of range
> >>>> +values will be converted to 2. Note that the event value just
> >>>> +indicates a probable level, but is not precise.
> >>>> +
> >>>> +2. Tx path buffer control
> >>>> +-------------------------
> >>>> +
> >>>> +Following files are provided to tune the buffer of tx path of the PCIe core.
> >>>> +
> >>>> +- tx_path_rx_req_alloc_buf_level: watermark of Rx requested
> >>>> +- tx_path_tx_req_alloc_buf_level: watermark of Tx requested
> >>>> +
> >>>> +These events influence the watermark of the buffer allocated for each
> >>>> +type. Rx means the inbound while Tx means outbound. The packets will
> >>>> +be stored in the buffer first and then transmitted either when the
> >>>> +watermark reached or when timed out. For a busy direction, you should
> >>>> +increase the related buffer watermark to avoid frequently posting and
> >>>> +thus enhance the performance. In most cases just keep the default value.
> >>>> +
> >>>> +The available tune data of above events is [0, 1, 2].
> >>>> +Writing a negative value will return an error, and out of range
> >>>> +values will be converted to 2. Note that the event value just
> >>>> +indicates a probable level, but is not precise.
> >>>
> >>> This is useful documentation but it also should be found in the ABI
> >>> documentation referred to above.
> >>>
> >>>> +
> >>>> +Trace
> >>>> +=====
> >>>> +
> >>>> +PTT trace is designed for dumping the TLP headers to the memory, which
> >>>> +can be used to analyze the transactions and usage condition of the PCIe
> >>>> +Link. You can choose to filter the traced headers by either requester ID,
> >>>> +or those downstream of a set of Root Ports on the same core of the PTT
> >>>> +device. It's also supported to trace the headers of certain type and of
> >>>> +certain direction.
> >>>> +
> >>>> +You can use the perf command `perf record` to set the parameters, start
> >>>> +trace and get the data. It's also supported to decode the trace
> >>>> +data with `perf report`. The control parameters for trace is inputted
> >>>> +as event code for each events, which will be further illustrated later.
> >>>> +An example usage is like
> >>>> +::
> >>>> +
> >>>> +    $ perf record -e hisi_ptt0_2/filter=0x80001,type=1,direction=1,
> >>>> +      format=1/ -- sleep 5
> >>>> +
> >>>> +This will trace the TLP headers downstream root port 0000:00:10.1 (event
> >>>> +code for event 'filter' is 0x80001) with type of posted TLP requests,
> >>>> +direction of inbound and traced data format of 8DW.
> >>>> +
> >>>> +1. filter
> >>>> +---------
> >>>> +
> >>>> +The TLP headers to trace can be filtered by the Root Ports or the requester
> >>>> +ID of the endpoints, which are located on the same core of the PTT device.
> >>>> +You can set the filter by specifying the `filter` parameter which is required
> >>>> +to start the trace. The parameter value is 20 bit. The supported filters and
> >>>> +related values are outputted through `available_root_port_filters` and
> >>>> +`available_requester_filters` sysfs attributes for Root Ports and Requesters
> >>>> +respectively.
> >>>> +::
> >>>> +
> >>>> +    $ cat available_root_port_filters
> >>>> +    0000:00:10.0  0x80001
> >>>> +    0000:00:11.0  0x80004
> >>>> +    $ cat available_requester_filters
> >>>> +    0000:01:00.0  0x00100
> >>>> +    0000:01:00.1  0x00101
> >>>
> >>> If I remember correctly, one of the rule for sysfs is one line per entry.
> >>>
> >>
> >> Since one PTT devices may support several Root Ports and Endpoints on its core, I find no better
> >> way to make this information convenient and easy to use for the users to collect. So maybe this
> >> canbe an exception and there seems to have some limited examples like
> >> /sys/devices/system/node/node<N>/{meminfo, vmstat, meminfo}.
> >
> > You can either find a better solution or argue the matter with Greg.  I suggest
> > to introduce new directories, i.e "root_port_filters" and "requested_filters"
> > and under those have entries like "port0", "port1" and so on.
> >
>
> Thanks for the suggestion and it does make sense to me. But I've tried it and met some problems.
> I intended to create two attribute groups for each kind of filter and create attribute for each
> available filter. like:
>
> $ ls root_port_filters    # each filter exists as an sysfs attribute
> 0000:00:10.0 0000:00:11.0
> $ cat root_port_filters/0000:00:10.0    # read the attribute get the filter value
> 0x80001
>
> In pratice we may have no filter for certain types, for example no endpoint found on this PCIe
> core then we'll have no filter attributes under requester_filters directory. Or if endpoint
> devices are hot removed or manually removed through sysfs by the user, the filter attributes
> will be created/destroyed dynamically (will support following this series, an implementation
> will be like in [1]) which may make the directory empty. But sysfs won't allow to have an
> empty attribute group, restricted in [2].
>

My comment was not meant to provide a solution but to highlight
alternative options.  There are a lot of sysfs examples in the kernel
for you to look at.

> [1] https://lore.kernel.org/lkml/20220407125841.3678-4-yangyicong@hisilicon.com/
> [2] https://github.com/torvalds/linux/blob/5cf3bb0d3a2d0de94f3f551f0e4211068818aabf/fs/sysfs/group.c#L121
>
> >>
> >>>> +
> >>>> +Note that multiple Root Ports can be specified at one time, but only
> >>>> +one Endpoint function can be specified in one trace. Specifying both
> >>>> +Root Port and function at the same time is not supported.
> >>>> +
> >>>> +If no filter is available, reading the related filter sysfs attribute
> >>>> +will get an empty string.
> >>>> +::
> >>>> +
> >>>> +    $ cat available_root_port_filters
> >>>> +
> >>>> +    $ cat available_requester_filters
> >>>
> >>> Those too look overly long, and where to find them is not documented.  As such
> >>> users have to guest that it must be somewhere under
> >>> /sys/devices/hisi_ptt<sicl_id>_<core_id>/.
> >>>
> >>
> >> Since Root Port and Requester are PCIe terminologies so it's better to have them
> >> embedded to make it clear. Maybe 'available' can be removed.
> >>
> >> Will have all these sysfs attributes documented.
> >>
> >>> More comments tomorrow.
> >>>
> >>
> >> Thanks,
> >> Yicong
> > .
> >