On 2023/8/1 19:46, Yicong Yang wrote:

On 2023/7/28 20:41, Shuai Xue wrote:

quoted

On 2023/7/27 17:39, Jonathan Cameron wrote:

quoted

On Tue, 6 Jun 2023 23:14:07 +0800
Yicong Yang [off-list ref] wrote:

quoted

On 2023/6/6 15:49, Shuai Xue wrote:

quoted

This commit adds the PCIe Performance Monitoring Unit (PMU) driver support
for T-Head Yitian SoC chip. Yitian is based on the Synopsys PCI Express
Core controller IP which provides statistics feature. The PMU is not a PCIe
Root Complex integrated End Point(RCiEP) device but only register counters
provided by each PCIe Root Port.

To facilitate collection of statistics the controller provides the
following two features for each Root Port:

- Time Based Analysis (RX/TX data throughput and time spent in each
  low-power LTSSM state)
- Event counters (Error and Non-Error for lanes)

Note, only one counter for each type and does not overflow interrupt.

This driver adds PMU devices for each PCIe Root Port. And the PMU device is
named based the BDF of Root Port. For example,

    30:03.0 PCI bridge: Device 1ded:8000 (rev 01)

the PMU device name for this Root Port is dwc_rootport_3018.

Example usage of counting PCIe RX TLP data payload (Units of 16 bytes)::

    $# perf stat -a -e dwc_rootport_3018/Rx_PCIe_TLP_Data_Payload/

average RX bandwidth can be calculated like this:

    PCIe TX Bandwidth = PCIE_TX_DATA * 16B / Measure_Time_Window

Signed-off-by: Shuai Xue <xueshuai@linux.alibaba.com>
Reported-by: kernel test robot <redacted>
Link: https://lore.kernel.org/oe-kbuild-all/202305170639.XU3djFZX-lkp@intel.com/ (local)
Reviewed-by: Baolin Wang <baolin.wang@linux.alibaba.com>

I'll review on top to avoid any duplication with Yicong.

Thank you! It also served as a reminder that I missed Yicong's email. It appears
that Thunderbird mistakenly moved his email to the junk folder, resulting in me
overlooking it.

quoted

Note I've cropped the stuff neither of us commented on so it's
easier to spot the feedback.

Thank you for noting that. My feedback is replied inline.

quoted

Jonathan

quoted

quoted

---
 drivers/perf/Kconfig        |   7 +
 drivers/perf/Makefile       |   1 +
 drivers/perf/dwc_pcie_pmu.c | 706 ++++++++++++++++++++++++++++++++++++
 3 files changed, 714 insertions(+)
 create mode 100644 drivers/perf/dwc_pcie_pmu.c

diff --git a/drivers/perf/Kconfig b/drivers/perf/Kconfig
index 711f82400086..6ff3921d7a62 100644
--- a/drivers/perf/Kconfig
+++ b/drivers/perf/Kconfig

@@ -209,6 +209,13 @@ config MARVELL_CN10K_DDR_PMU
 	  Enable perf support for Marvell DDR Performance monitoring
 	  event on CN10K platform.
 
+config DWC_PCIE_PMU
+	tristate "Enable Synopsys DesignWare PCIe PMU Support"
+	depends on (ARM64 && PCI)
+	help
+	  Enable perf support for Synopsys DesignWare PCIe PMU Performance
+	  monitoring event on Yitian 710 platform.

The documentation kind of implies this isn't platform specific.
If some parts are (such as which events exist) then you may want to push
that to userspace / perftool with appropriate matching against specific SoC.

If it is generic, then change this text to "event on platform including the Yitian 710."

It is generic without any platform specific, so I will change it as you expected.

quoted

quoted

quoted

+
 source "drivers/perf/arm_cspmu/Kconfig"
 
 source "drivers/perf/amlogic/Kconfig"

quoted

quoted

new file mode 100644
index 000000000000..8bfcf6e0662d

--- /dev/null
+++ b/drivers/perf/dwc_pcie_pmu.c

@@ -0,0 +1,706 @@

...

quoted

quoted

+
+struct dwc_pcie_pmu {
+	struct pci_dev		*pdev;		/* Root Port device */  

If the root port removed after the probe of this PCIe PMU driver, we'll access the NULL
pointer. I didn't see you hold the root port to avoid the removal.

Do you mean that I should have a reference count of rootport by pci_dev_get() when allocating
pcie_pmu?

     pcie_pmu->pdev = pci_dev_get();

It could be one option, but will block the removal of device from userspace. Another option
is to register a PCI bus notifier then on removal/added the driver can get notified and handle
it, for example, remove the related PMU on the removal of the root ports.

I see, but can root port be removed from userspace? I check the hotplug slot interface, no root
port is available to power off.

Thank you.

Best Regards,
Shuai

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel