[v10,0/5] drivers/perf: add Synopsys DesignWare PCIe PMU driver support

  Change Log
==========

- move the &plat_dev->dev to previous line to warp more beautiful (Per Jonathan)
- rename error label with the same suffix 'err'  (Per Jonathan)
- drop unnecessary else branch and return directly (Per Baolin)
- warp out set prev_count from dwc_pcie_pmu_set_period (Per Baolin)
- use PMU_FORMAT_ATTR to simplify format sysfs stuff (Per Will)
- export pci_clear_and_set_dword() to simplify _enable() functions  (Per Will)
- simplify _read() function by unconditionally calculate with unit in bytes plused if branch for group#1 event (Per Will and Robin)
- simplify _update() function by unconditionally mask with 64-bit width plused if branch for lane event (Per Will)
- add type sanity check in _init() (Per Will)
- test with fuzzing tool before this new version (Per Will)
- register a platform device for each PCI device to probe RAS_DES PMU cap (Per Robin)
- add dwc_pcie_vendor_ids to extend vendor id for future added device (Per Krishna)
- pickup review-by tag from Baolin, Yicong and Jonathan 


changes since v8:
- tidy up doc and pick up Reviewed-by tag in pathc 1/4 (Per Yicong and Jonathan)
- rename ras_des as ras_des_offset and remove coment about it (Per Jonathan)
- keep exact reverse order of what happened in probe when unregister pmu (Per Jonathan)
- remove the hender length check to make the driver more compatible (Per Jonathan)
- move unwind of register pmu forward to fix list_del() on something that was never added (Per Jonathan)
- use devm_add_action_or_reset() to unwind bus_register_notifier() (Per Jonathan)
- use local pdev directly in dwc_pcie_pmu_offline_cpu() and dwc_pcie_pmu_probe() (Per Jonathan)
- use the approach of a gotos to handle errors in dwc_pcie_pmu_init() (Per Jonathan)
- multiply the counter value at point of read by 16 for group#1 events (Per Jonathan)

changes since v7:
- add config help with alibaba name (per Bjorn)
- remove the ARM64 dependency (per Bjorn and Jonathan)
- fix typo and column warp (per Bjorn) 
- move list_del() after perf_pmu_unregister() (per Bjorn) 
- reorder the funtions by interests (per Bjorn)
- rewrite commit log about PMU counters, also update doc (per Bjorn)
- extend to support stat time-based analysis and lane event at the same time (per Bjorn and Jonathan)
Link: https://lore.kernel.org/linux-arm-kernel/20231012032856.2640-2-xueshuai@linux.alibaba.com/T/

changes since v6:
- improve editorial things in doc (Per Jonathan)
- change config help to generic text (Per Jonathan)
- remove macro to_dwc_pcie_pmu by moving pmu as the first member to struct dwc_pcie_pmu (Per Yicong)
- add event type check in dwc_pcie_event_show() to keep consistent with other function (Per Jonathan)
- remove intended blank line (Per Yicong)
- protect against lower 32 bits of counter overflow by try again trick (Per Jonathan)
- call pci_dev_put on all the return branch to keep the refcnt balance (Per Jonathan and Yicong)
- use devm_add_action_or_reset() to automatic unwind (Per Jonathan)
- fix picking numa-aware context cpu up when offline and offline cpu (Per Jonathan)
- simplify online cpu by init pcie_pmu->on_cpu as -1 (Per Jonathan)
- add bus_register_notifier() to handle rootport hotplug (Per Yicong)
- pick up Acked-by from Bjorn for patch 2/4 (Per Bjorn)
Link: https://lore.kernel.org/lkml/20230606074938.97724-1-xueshuai@linux.alibaba.com/T/

changes since v5:
- Rewrite the commit log to follow policy in pci_ids.h (Bjorn Helgaas)
- return error code when __dwc_pcie_pmu_probe failed (Baolin Wang)
- call 'cpuhp_remove_multi_state()' when exiting the driver. (Baolin Wang)
- pick up Review-by tag from Baolin for Patch 1 and 3
Link: https://lore.kernel.org/lkml/ZGuSimj1cuQl3W5L@bhelgaas/T/#mba3fa2572dde0deddb40b5b24a31f4df41004bdf

changes since v4:

1. addressing commens from Bjorn Helgaas:
- reorder the includes by alpha
- change all macros with upper-case hex
- change ras_des type into u16
- remove unnecessary outer "()"
- minor format changes

2. Address commensts from Jonathan Cameron:
- rewrite doc and add a example to show how to use lane event

3. fix compile error reported by: kernel test robot
- remove COMPILE_TEST and add depend on PCI in kconfig
- add Reported-by: kernel test robot <lkp@intel.com>

Changes since v3:

1. addressing comments from Robin Murphy:
- add a prepare patch to define pci id in linux/pci_ids.h
- remove unnecessary 64BIT dependency
- fix DWC_PCIE_PER_EVENT_OFF/ON macro
- remove dwc_pcie_pmu struct and move all its fileds into dwc_pcie_rp_info
- remove unnecessary format field show
- use sysfs_emit() instead of all the assorted sprintf() and snprintf() calls.
- remove unnecessary spaces and remove unnecessary cast to follow event show convention
- remove pcie_pmu_event_attr_is_visible
- fix a refcout leak on error branch when walk pci device in for_each_pci_dev
- remove bdf field from dwc_pcie_rp_info and calculate it at runtime
- finish all the checks before allocating rp_info to avoid hanging wasted memory
- remove some unused fields
- warp out control register configuration from sub function to .add()
- make function return type with a proper signature
- fix lane event count enable by clear DWC_PCIE_CNT_ENABLE field first
- pass rp_info directly to the read_*_counter helpers and in start, stop and add callbacks
- move event type validtion into .event_init()
- use is_sampling_event() to be consistent with everything else of pmu drivers
- remove unnecessary dev_err message in .event_init()
- return EINVAL instead EOPNOTSUPP for not a valid event 
- finish all the checks before start modifying the event
- fix sibling event check by comparing event->pmu with sibling->pmu
- probe PMU for each rootport independently
- use .update() as .read() directly
- remove dynamically generating symbolic name of lane event
- redefine static symbolic name of lane event and leave lane filed to user
- add CPU hotplug support

2. addressing comments from Baolin:
- add a mask to avoid possible overflow

Changes since v2 addressing comments from Baolin:
- remove redundant macro definitions
- use dev_err to print error message
- change pmu_is_register to boolean
- use PLATFORM_DEVID_NONE macro
- fix module author format

Changes since v1:

1. address comments from Jonathan:
- drop marco for PMU name and VSEC version
- simplify code with PCI standard marco
- simplify code with FIELD_PREP()/FIELD_GET() to replace shift marco
- name register filed with single _ instead double
- wrap dwc_pcie_pmu_{write}_dword out and drop meaningless snaity check 
- check vendor id while matching vesc with pci_find_vsec_capability()
- remove RP_NUM_MAX and use a list to organize PMU devices for rootports
- replace DWC_PCIE_CREATE_BDF with standard PCI_DEVID
- comments on riping register together

2. address comments from Bjorn:
- rename DWC_PCIE_VSEC_ID to DWC_PCIE_VSEC_RAS_DES_ID
- rename cap_pos to ras_des
- simplify declare of device_attribute with DEVICE_ATTR_RO
- simplify code with PCI standard macro and API like pcie_get_width_cap()
- fix some code style problem and typo
- drop meaningless snaity check of container_of

3. address comments from Yicong:
- use sysfs_emit() to replace sprintf()
- simplify iteration of pci device with for_each_pci_dev
- pick preferred CPUs on a near die and add comments
- unregister PMU drivers only for failed ones
- log on behalf PMU device and give more hint
- fix some code style problem

(Thanks for all comments and they are very valuable to me)

Shuai Xue (5):
  docs: perf: Add description for Synopsys DesignWare PCIe PMU driver
  PCI: Add Alibaba Vendor ID to linux/pci_ids.h
  PCI: move pci_clear_and_set_dword helper to pci header
  drivers/perf: add DesignWare PCIe PMU driver
  MAINTAINERS: add maintainers for DesignWare PCIe PMU driver

 .../admin-guide/perf/dwc_pcie_pmu.rst         |  94 +++
 Documentation/admin-guide/perf/index.rst      |   1 +
 MAINTAINERS                                   |   7 +
 drivers/infiniband/hw/erdma/erdma_hw.h        |   2 -
 drivers/pci/access.c                          |  12 +
 drivers/pci/pcie/aspm.c                       |  11 -
 drivers/perf/Kconfig                          |   7 +
 drivers/perf/Makefile                         |   1 +
 drivers/perf/dwc_pcie_pmu.c                   | 798 ++++++++++++++++++
 include/linux/pci.h                           |   2 +
 include/linux/pci_ids.h                       |   2 +
 11 files changed, 924 insertions(+), 13 deletions(-)
 create mode 100644 Documentation/admin-guide/perf/dwc_pcie_pmu.rst
 create mode 100644 drivers/perf/dwc_pcie_pmu.c
  

Hi Shuai,

On Sat, 4 Nov 2023, Shuai Xue wrote:
> Change Log
> ==========
>
> - move the &plat_dev->dev to previous line to warp more beautiful (Per Jonathan)
> - rename error label with the same suffix 'err'  (Per Jonathan)
> - drop unnecessary else branch and return directly (Per Baolin)
> - warp out set prev_count from dwc_pcie_pmu_set_period (Per Baolin)
> - use PMU_FORMAT_ATTR to simplify format sysfs stuff (Per Will)
> - export pci_clear_and_set_dword() to simplify _enable() functions  (Per Will)
> - simplify _read() function by unconditionally calculate with unit in bytes plused if branch for group#1 event (Per Will and Robin)
> - simplify _update() function by unconditionally mask with 64-bit width plused if branch for lane event (Per Will)
> - add type sanity check in _init() (Per Will)
> - test with fuzzing tool before this new version (Per Will)
> - register a platform device for each PCI device to probe RAS_DES PMU cap (Per Robin)
> - add dwc_pcie_vendor_ids to extend vendor id for future added device (Per Krishna)
> - pickup review-by tag from Baolin, Yicong and Jonathan

...

> Shuai Xue (5):
>  docs: perf: Add description for Synopsys DesignWare PCIe PMU driver
>  PCI: Add Alibaba Vendor ID to linux/pci_ids.h
>  PCI: move pci_clear_and_set_dword helper to pci header
>  drivers/perf: add DesignWare PCIe PMU driver
>  MAINTAINERS: add maintainers for DesignWare PCIe PMU driver

As I mentioned earlier, I successfully tested your patchset with a few 
patches on top of it to enable DWC PCIe PMU on AmpereOne. Thus, feel free 
to add this tag to all the patches above:

 	Tested-by: Ilkka Koskinen <ilkka@os.amperecomputing.com>

Br, Ilkka


>
> .../admin-guide/perf/dwc_pcie_pmu.rst         |  94 +++
> Documentation/admin-guide/perf/index.rst      |   1 +
> MAINTAINERS                                   |   7 +
> drivers/infiniband/hw/erdma/erdma_hw.h        |   2 -
> drivers/pci/access.c                          |  12 +
> drivers/pci/pcie/aspm.c                       |  11 -
> drivers/perf/Kconfig                          |   7 +
> drivers/perf/Makefile                         |   1 +
> drivers/perf/dwc_pcie_pmu.c                   | 798 ++++++++++++++++++
> include/linux/pci.h                           |   2 +
> include/linux/pci_ids.h                       |   2 +
> 11 files changed, 924 insertions(+), 13 deletions(-)
> create mode 100644 Documentation/admin-guide/perf/dwc_pcie_pmu.rst
> create mode 100644 drivers/perf/dwc_pcie_pmu.c
>
> -- 
> 2.39.3
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
  

On 2023/11/16 08:57, Ilkka Koskinen wrote:
> 
> Hi Shuai,
> 
> On Sat, 4 Nov 2023, Shuai Xue wrote:
>> Change Log
>> ==========
>>
>> - move the &plat_dev->dev to previous line to warp more beautiful (Per Jonathan)
>> - rename error label with the same suffix 'err'  (Per Jonathan)
>> - drop unnecessary else branch and return directly (Per Baolin)
>> - warp out set prev_count from dwc_pcie_pmu_set_period (Per Baolin)
>> - use PMU_FORMAT_ATTR to simplify format sysfs stuff (Per Will)
>> - export pci_clear_and_set_dword() to simplify _enable() functions  (Per Will)
>> - simplify _read() function by unconditionally calculate with unit in bytes plused if branch for group#1 event (Per Will and Robin)
>> - simplify _update() function by unconditionally mask with 64-bit width plused if branch for lane event (Per Will)
>> - add type sanity check in _init() (Per Will)
>> - test with fuzzing tool before this new version (Per Will)
>> - register a platform device for each PCI device to probe RAS_DES PMU cap (Per Robin)
>> - add dwc_pcie_vendor_ids to extend vendor id for future added device (Per Krishna)
>> - pickup review-by tag from Baolin, Yicong and Jonathan
> 
> ...
> 
>> Shuai Xue (5):
>>  docs: perf: Add description for Synopsys DesignWare PCIe PMU driver
>>  PCI: Add Alibaba Vendor ID to linux/pci_ids.h
>>  PCI: move pci_clear_and_set_dword helper to pci header
>>  drivers/perf: add DesignWare PCIe PMU driver
>>  MAINTAINERS: add maintainers for DesignWare PCIe PMU driver
> 
> As I mentioned earlier, I successfully tested your patchset with a few patches on top of it to enable DWC PCIe PMU on AmpereOne. Thus, feel free to add this tag to all the patches above:
> 
>     Tested-by: Ilkka Koskinen <ilkka@os.amperecomputing.com>
> 
> Br, Ilkka

Hi, Ilkka,

Thank you.

I will add your tested-by tag in the upcoming version v11. However, I
kindly request some time to wait for feedback from esteemed maintainers such
as Will, and Robin.

Cheers,
Shuai
  

On Thu, 16 Nov 2023, Shuai Xue wrote:
> On 2023/11/16 08:57, Ilkka Koskinen wrote:
>>
>> Hi Shuai,
>>
>> On Sat, 4 Nov 2023, Shuai Xue wrote:
>>> Change Log
>>> ==========
>>>
>>> - move the &plat_dev->dev to previous line to warp more beautiful (Per Jonathan)
>>> - rename error label with the same suffix 'err'  (Per Jonathan)
>>> - drop unnecessary else branch and return directly (Per Baolin)
>>> - warp out set prev_count from dwc_pcie_pmu_set_period (Per Baolin)
>>> - use PMU_FORMAT_ATTR to simplify format sysfs stuff (Per Will)
>>> - export pci_clear_and_set_dword() to simplify _enable() functions  (Per Will)
>>> - simplify _read() function by unconditionally calculate with unit in bytes plused if branch for group#1 event (Per Will and Robin)
>>> - simplify _update() function by unconditionally mask with 64-bit width plused if branch for lane event (Per Will)
>>> - add type sanity check in _init() (Per Will)
>>> - test with fuzzing tool before this new version (Per Will)
>>> - register a platform device for each PCI device to probe RAS_DES PMU cap (Per Robin)
>>> - add dwc_pcie_vendor_ids to extend vendor id for future added device (Per Krishna)
>>> - pickup review-by tag from Baolin, Yicong and Jonathan
>>
>> ...
>>
>>> Shuai Xue (5):
>>>  docs: perf: Add description for Synopsys DesignWare PCIe PMU driver
>>>  PCI: Add Alibaba Vendor ID to linux/pci_ids.h
>>>  PCI: move pci_clear_and_set_dword helper to pci header
>>>  drivers/perf: add DesignWare PCIe PMU driver
>>>  MAINTAINERS: add maintainers for DesignWare PCIe PMU driver
>>
>> As I mentioned earlier, I successfully tested your patchset with a few patches on top of it to enable DWC PCIe PMU on AmpereOne. Thus, feel free to add this tag to all the patches above:
>>
>>     Tested-by: Ilkka Koskinen <ilkka@os.amperecomputing.com>
>>
>> Br, Ilkka
>
> Hi, Ilkka,
>
> Thank you.
>
> I will add your tested-by tag in the upcoming version v11. However, I
> kindly request some time to wait for feedback from esteemed maintainers such
> as Will, and Robin.

Sounds good. Actually, I can give you a little feedback on the driver 
patch meanwhile.

--Ilkka

>
> Cheers,
> Shuai
>