[RFC PATCH v3 1/2] drivers/perf: hisi: Add description for HNS3 PMU driver

[huangguangbin (A)]

On 2021/5/10 23:48, Jonathan Cameron wrote:
> On Sat, 8 May 2021 17:44:55 +0800
> Guangbin Huang <huangguangbin2 at huawei.com> wrote:
> 
>> HNS3 PMU End Point device is supported on HiSilicon HIP09 platform, so
>> add document hns3-pmu.rst to provide guidance on how to use it.
>>
>> Signed-off-by: Guangbin Huang <huangguangbin2 at huawei.com>
> 
> A few question and suggestions inline.
> 
> Thanks,
> 
> Jonathan
> 
Thank you, Jonathan!

>> ---
>>   Documentation/admin-guide/perf/hns3-pmu.rst | 129 ++++++++++++++++++++++++++++
>>   1 file changed, 129 insertions(+)
>>   create mode 100644 Documentation/admin-guide/perf/hns3-pmu.rst
>>
>> diff --git a/Documentation/admin-guide/perf/hns3-pmu.rst b/Documentation/admin-guide/perf/hns3-pmu.rst
>> new file mode 100644
>> index 0000000..9695688
>> --- /dev/null
>> +++ b/Documentation/admin-guide/perf/hns3-pmu.rst
>> @@ -0,0 +1,129 @@
>> +======================================
>> +HNS3 Performance Monitoring Unit (PMU)
>> +======================================
>> +
>> +HNS3(HiSilicon network system 3) Performance Monitoring Unit (PMU) is an
>> +End Point device to collect performance statistics of HiSilicon SoC NIC.
>> +On Hip09, each SICL(Super I/O cluster) has one PMU device.
>> +
>> +HNS3 PMU is supported to collect performance statistics of bandwidth,
> 
> Perhaps replace with:
> 
> HNS3 PMU supports collection of performance statistics such as bandwidth,
> latency, packet rate and interrupt rate.
> 
Ok.

>> +latency, packet rate and interrupt rate.
>> +
>> +Each HNS3 PMU supports up to 8 hardware events.
> 
> Do we have hardware versions that support less?  If not replace with:
> 
> Each HNS3 PMU supports 8 hardware events.
> 
Ok.

>> +
>> +HNS3 PMU driver
>> +===============
>> +
>> +The HNS3 PMU driver registers a perf PMU with the name of its device id.::
>> +
>> +  /sys/devices/hns3_pmu_<device_id>
>> +
>> +The device_id is read from hardware register, it contains information of
>> +chip_id(bit 31:2) and SICL_ID(bit 1:0). One chip may have one or more SICL.
>> +
>> +PMU driver provides description of available events, filter modes, format,
>> +identifier and cpumask in sysfs.
>> +
>> +The "events" directory describes the event code and subevent code of all
>> +supported events shown in perf list.
>> +
>> +The "filtermode" directory describes the supported filter modes of each
>> +event.
>> +
>> +The "format" directory describes all formats of the config (events) and
>> +config1 (filter options) fields of the perf_event_attr structure.
>> +
>> +The "identifier" file shows version of PMU hardware device.
>> +
>> +Example usage of checking event code and subevent code::
>> +
>> +  $# cat /sys/devices/hns3_pmu_0/events/bw_igu_ssu
>> +  config=0x0000
>> +
>> +The upper 8 bits of config is event code, lower 8 bits of config is
>> +subevent code.
> 
> Could you use an example that isn't 0x0000?   That way you could illustrate
> this comment by saying something like:
> 
>     config=0x0103
> 
> The upper 8 bits of config (here 0x01) are the event code. The lower 8 bits
> of config (here 0x03) are the subevent code.
> 
Ok, thanks your suggestion.

>> +
>> +Example usage of checking supported filter mode::
>> +
>> +  $# cat /sys/devices/hns3_pmu_0/filtermode/bw_igu_ssu
>> +  filter mode supported: global/port/port-tc/
> 
> This formatting seems unusual for a sysfs attribute are there similar
> examples already in use?
> 
I can't find similar example, but I need to show supported filter mode of
every event for user. If this formatting is unacceptable, I don't know how
to tell these information to user.

>> +
>> +Example usage of perf::
>> +
>> +  $# perf list
>> +  hns3_pmu_0/bw_igu_ssu/ [kernel PMU event]
>> +  ------------------------------------------
>> +
>> +  $# perf stat -a -e hns3_pmu_0/bw_igu_ssu,global=1/ -I 1000
>> +  or
>> +  $# perf stat -a -e hns3_pmu_0/event=0,subevent=0,global=1/ -I 1000
>> +
>> +The current driver does not support sampling. So "perf record" is unsupported.
> 
> One sentence:
> The current driver does not support sampling, so "perf record" is unsupported.
> 
> Given this is effectively an uncore device, I'd imagine there is no sensible way
> of supporting sampling?  In which case I'd just say it is not supported.
> 
Ok, I will modify this sentence in next version.

>> +Also attach to a task is unsupported for HNS3 PMU.
>> +
>> +Filter modes
>> +--------------
>> +
>> +1. global mode
>> +PMU collect performance statistic of all functions of IO DIE. Set the
> 
> statistics for all
> 
Ok.

>> +"global" filter option to 1 will enable this mode.
> 
> I'm assuming HNS related functions of the IO die?  Perhaps "HNS3 PCI functions"
> is worth saying rather than just "functions"
> 
Yes, I will modify this description.

>> +Example usage of perf::
>> +
>> +  $# perf stat -a -e hns3_pmu_0/event=0,subevent=0,global=1/ -I 1000
>> +
>> +2. port mode
>> +PMU collect performance statistic of one whole physical port. The port id
>> +is same as mac id. The "tc" filter option must be set to 0xF in this mode.
>> +Example usage of perf::
>> +
>> +  $# perf stat -a -e hns3_pmu_0/event=0,subevent=0,port=0,tc=0xF/ -I 1000
>> +
>> +3. port-tc mode
>> +PMU collect performance statistic of one tc of physical port. The port id
>> +is same as mac id. The "tc" filter option must be set to 0 ~ 7 in this
>> +mode.
> 
> Say what tc stands for - I would assume traffic class?
> 
Yes, tc stands for traffic class. I will add it.

>> +Example usage of perf::
>> +
>> +  $# perf stat -a -e hns3_pmu_0/event=0,subevent=0,port=0,tc=0/ -I 1000
>> +
>> +4. func mode
>> +PMU collect performance statistic of one PF/VF. The function id is BDF of
>> +PF/VF, its conversion formula::
>> +
>> +  func = (bus << 8) + (device << 3) + (function)
>> +
>> +for example:
>> +  BDF         func
>> +  35:00.0    0x3500
>> +  35:00.1    0x3501
>> +  35:01.0    0x3508
>> +
>> +In this mode, the "queue" filter option must be set to 0xFFFF.
>> +Example usage of perf::
>> +
>> +  $# perf stat -a -e hns3_pmu_0/event=0,subevent=0,bdf=0x3500,queue=0xFFFF/ -I 1000
>> +
>> +5. func-queue mode
>> +PMU collect performance statistic of one queue of PF/VF. The function id
>> +is BDF of PF/VF, its conversion formula::
>> +
>> +  func = (bus << 8) + (device << 3) + (function)
>> +
>> +In this mode, the "queue" filter option must be set to the exact queue id
>> +of function.
>> +Example usage of perf::
>> +
>> +  $# perf stat -a -e hns3_pmu_0/event=0,subevent=0,bdf=0x3500,queue=0/ -I 1000
>> +
>> +
> 
> Drop one blank line here for consistency
> 
Ok, thanks.

>> +6. func-intr mode
>> +PMU collect performance statistic of one interrupt of PF/VF. The function
>> +id is BDF of PF/VF, its conversion formula::
>> +
>> +  func = (bus << 8) + (device << 3) + (function)
>> +
>> +In this mode, the "intr" filter option must be set to the exact interrupt
>> +id of function.
>> +Example usage of perf::
>> +
>> +  $# perf stat -a -e hns3_pmu_0/event=0,subevent=0,bdf=0x3500,intr=0/ -I 1000
> 
> .
>