On Fri, 20 Oct 2023 21:42:29 +0800
Shuai Xue [off-list ref] wrote:

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 a PCIe
configuration space register block provided by each PCIe Root Port in a
Vendor-Specific Extended Capability named RAS D.E.S (Debug, Error
injection, and Statistics).

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

- one 64-bit counter for Time Based Analysis (RX/TX data throughput and
  time spent in each low-power LTSSM state) and
- one 32-bit counter for Event Counting (error and non-error events for
  a specified lane)

Note: There is no interrupt for counter overflow.

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 bytes)::

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

average RX bandwidth can be calculated like this:

    PCIe TX Bandwidth = Rx_PCIe_TLP_Data_Payload / Measure_Time_Window

Signed-off-by: Shuai Xue <xueshuai@linux.alibaba.com>

LGTM other than some really trivial stuff inline if you are doing a v10

Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

+static int dwc_pcie_pmu_probe(struct platform_device *plat_dev)
+{
+	struct pci_dev *pdev = NULL;
+	struct dwc_pcie_pmu *pcie_pmu;
+	bool notify = false;
+	char *name;
+	u32 bdf;
+	int ret;
+
+	/* Match the rootport with VSEC_RAS_DES_ID, and register a PMU for it */
+	for_each_pci_dev(pdev) {
+		u16 vsec;
+		u32 val;
+
+		if (!(pci_is_pcie(pdev) &&
+		      pci_pcie_type(pdev) == PCI_EXP_TYPE_ROOT_PORT))
+			continue;
+
+		vsec = pci_find_vsec_capability(pdev, PCI_VENDOR_ID_ALIBABA,
+						DWC_PCIE_VSEC_RAS_DES_ID);
+		if (!vsec)
+			continue;
+
+		pci_read_config_dword(pdev, vsec + PCI_VNDR_HEADER, &val);
+		if (PCI_VNDR_HEADER_REV(val) != 0x04)
+			continue;
+		pci_dbg(pdev,
+			"Detected PCIe Vendor-Specific Extended Capability RAS DES\n");
+
+		bdf = PCI_DEVID(pdev->bus->number, pdev->devfn);
+		name = devm_kasprintf(&plat_dev->dev, GFP_KERNEL, "dwc_rootport_%x",
+				      bdf);
+		if (!name) {
+			ret = -ENOMEM;
+			goto out;
+		}
+
+		/* All checks passed, go go go */
+		pcie_pmu = devm_kzalloc(&plat_dev->dev, sizeof(*pcie_pmu), GFP_KERNEL);
+		if (!pcie_pmu) {
+			ret = -ENOMEM;
+			goto out;
+		}
+
+		pcie_pmu->pdev = pdev;
+		pcie_pmu->ras_des_offset = vsec;
+		pcie_pmu->nr_lanes = pcie_get_width_cap(pdev);
+		pcie_pmu->on_cpu = -1;
+		pcie_pmu->pmu = (struct pmu){
+			.module		= THIS_MODULE,
+			.attr_groups	= dwc_pcie_attr_groups,
+			.capabilities	= PERF_PMU_CAP_NO_EXCLUDE,
+			.task_ctx_nr	= perf_invalid_context,
+			.event_init	= dwc_pcie_pmu_event_init,
+			.add		= dwc_pcie_pmu_event_add,
+			.del		= dwc_pcie_pmu_event_del,
+			.start		= dwc_pcie_pmu_event_start,
+			.stop		= dwc_pcie_pmu_event_stop,
+			.read		= dwc_pcie_pmu_event_update,
+		};
+
+		/* Add this instance to the list used by the offline callback */
+		ret = cpuhp_state_add_instance(dwc_pcie_pmu_hp_state,
+					       &pcie_pmu->cpuhp_node);
+		if (ret) {
+			pci_err(pdev,
+				"Error %d registering hotplug @%x\n", ret, bdf);
+			goto out;
+		}
+
+		/* Unwind when platform driver removes */
+		ret = devm_add_action_or_reset(
+			&plat_dev->dev, dwc_pcie_pmu_remove_cpuhp_instance,
+			&pcie_pmu->cpuhp_node);
+		if (ret)
+			goto out;
+
+		ret = perf_pmu_register(&pcie_pmu->pmu, name, -1);
+		if (ret) {
+			pci_err(pdev,
+				"Error %d registering PMU @%x\n", ret, bdf);
+			goto out;
+		}
+
+		/* Cache PMU to handle pci device hotplug */
+		list_add(&pcie_pmu->pmu_node, &dwc_pcie_pmu_head);
+		pcie_pmu->registered = true;
+		notify = true;
+
+		ret = devm_add_action_or_reset(
+			&plat_dev->dev, dwc_pcie_pmu_unregister_pmu, pcie_pmu);

line wrapping is a bit ugly - I would move the &plat_dev->dev to previous line.
and I think you can get away with aligning the rest just after the (

+		if (ret)
+			goto out;
+	}
+
+	if (notify && !bus_register_notifier(&pci_bus_type, &dwc_pcie_pmu_nb))
+		return devm_add_action_or_reset(
+			&plat_dev->dev, dwc_pcie_pmu_unregister_nb, NULL);
+
+	return 0;
+
+out:
+	pci_dev_put(pdev);
+
+	return ret;
+}

+static int __init dwc_pcie_pmu_init(void)
+{
+	int ret;
+
+	ret = cpuhp_setup_state_multi(CPUHP_AP_ONLINE_DYN,
+				      "perf/dwc_pcie_pmu:online",
+				      dwc_pcie_pmu_online_cpu,
+				      dwc_pcie_pmu_offline_cpu);
+	if (ret < 0)
+		return ret;
+
+	dwc_pcie_pmu_hp_state = ret;
+
+	ret = platform_driver_register(&dwc_pcie_pmu_driver);
+	if (ret)
+		goto platform_driver_register_err;
+
+	dwc_pcie_pmu_dev = platform_device_register_simple(
+				"dwc_pcie_pmu", PLATFORM_DEVID_NONE, NULL, 0);
+	if (IS_ERR(dwc_pcie_pmu_dev)) {
+		ret = PTR_ERR(dwc_pcie_pmu_dev);
+		goto platform_device_register_error;
+	}
+
+	return 0;
+
+platform_device_register_error:

Trivial but I'd standardize on err or error, not mix them.

+	platform_driver_unregister(&dwc_pcie_pmu_driver);
+platform_driver_register_err:
+	cpuhp_remove_multi_state(dwc_pcie_pmu_hp_state);
+
+	return ret;
+}

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