[PATCH v3 0/4] New vendor support and optimizations for DWC PCIe PMU

[PATCH v3 0/4] New vendor support and optimizations for DWC PCIe PMU

[Yicong Yang]

This patchset mainly enable the DWC PCIe PMU on our platform (Patch 1/4).
Besides do some optimization and refactor:
- Patch 2/4 extend the monitoring limit on platforms with narrowed counter width (< 64 bit)
- Patch 3/4 switch the driver to use the faux device interface
- Patch 4/4 fix an old issue that PMU may counting when corresponding root port
  is runtime suspended.

Change since v2:
- Fix the sytle and ID orders per the comments.
- Fix one runtime pm issue
Link: https://lore.kernel.org/lkml/20260629092717.74946-1-yang.yicong@picoheart.com/

Change since v1:
- always enable hrtimer for updating narrowed counters on affected platform
- refine the commints and fix styles per Jonthan, thanks.
Link: https://lore.kernel.org/all/20260615063459.25361-1-yang.yicong@picoheart.com/


Yicong Yang (3):
  perf/dwc_pcie: Add support for Picoheart vendor devices
  perf/dwc_pcie: Convert to faux device interface
  drivers/perf: Fix the condition where the root port is runtime
    suspended

Yufan Dou (1):
  perf/dwc_pcie: Support narrowed time-based counter for long time
    monitoring

 drivers/perf/dwc_pcie_pmu.c | 195 +++++++++++++++++++++++++-----------
 include/linux/pci_ids.h     |   2 +
 include/linux/pcie-dwc.h    |   2 +
 3 files changed, 141 insertions(+), 58 deletions(-)

-- 
2.50.1 (Apple Git-155)

[PATCH v3 1/4] perf/dwc_pcie: Add support for Picoheart vendor devices

[Yicong Yang]

Add PCI_VENDOR_ID_PICOHEART in pci_ids.h. Update the DWC PCIe
vendor table with Picoheart PCIe Vendor ID to enable the PCIe
PMU support.

Acked-by: Bjorn Helgaas <bhelgaas@google.com>
Reviewed-by: Shuai Xue <xueshuai@linux.alibaba.com>
Signed-off-by: Yicong Yang <yang.yicong@picoheart.com>
---
 include/linux/pci_ids.h  | 2 ++
 include/linux/pcie-dwc.h | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h
index 1c9d40e09107..2c17239aacea 100644
--- a/include/linux/pci_ids.h
+++ b/include/linux/pci_ids.h
@@ -2640,6 +2640,8 @@
 #define PCI_VENDOR_ID_SUNIX		0x1fd4
 #define PCI_DEVICE_ID_SUNIX_1999	0x1999
 
+#define PCI_VENDOR_ID_PICOHEART		0x20fa
+
 #define PCI_VENDOR_ID_HINT             0x3388
 #define PCI_DEVICE_ID_HINT_VXPROII_IDE 0x8013
 
diff --git a/include/linux/pcie-dwc.h b/include/linux/pcie-dwc.h
index 8ff778e7aec0..d3cd701a58e3 100644
--- a/include/linux/pcie-dwc.h
+++ b/include/linux/pcie-dwc.h
@@ -26,6 +26,8 @@ static const struct dwc_pcie_vsec_id dwc_pcie_rasdes_vsec_ids[] = {
 	  .vsec_id = 0x02, .vsec_rev = 0x4 },
 	{ .vendor_id = PCI_VENDOR_ID_AMPERE,
 	  .vsec_id = 0x02, .vsec_rev = 0x4 },
+	{ .vendor_id = PCI_VENDOR_ID_PICOHEART,
+	  .vsec_id = 0x02, .vsec_rev = 0x4 },
 	{ .vendor_id = PCI_VENDOR_ID_QCOM,
 	  .vsec_id = 0x02, .vsec_rev = 0x4 },
 	{ .vendor_id = PCI_VENDOR_ID_ROCKCHIP,
-- 
2.50.1 (Apple Git-155)

Re: [PATCH v3 1/4] perf/dwc_pcie: Add support for Picoheart vendor devices

[sashiko-bot]

> Add PCI_VENDOR_ID_PICOHEART in pci_ids.h. Update the DWC PCIe
> vendor table with Picoheart PCIe Vendor ID to enable the PCIe
> PMU support.
> 
> Acked-by: Bjorn Helgaas <bhelgaas@google.com>
> Reviewed-by: Shuai Xue <xueshuai@linux.alibaba.com>
> Signed-off-by: Yicong Yang <yang.yicong@picoheart.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260708084507.98242-1-yang.yicong@picoheart.com?part=1

[PATCH v3 2/4] perf/dwc_pcie: Support narrowed time-based counter for long time monitoring

[Yicong Yang]

From: Yufan Dou <douyufan@picoheart.com>

The DWC PCIe Time-Based Analysis Data Register (the counter for time-based
events) is architected as 64-bit, but some hardware implementations do not
implement the full width.  On these implementations the counter stops after
reaching its implemented width. This will limit the usage for short time
monitoring only. The counter will only cover ~15s for monitoring RX TLP
payloads on our platform.

Add an optional hrtimer that fires every 2 seconds. It'll take the role
as the counter overflow interrupt to read-update-reset the counter and
event counts to break the limits of the narrow counters. It'll only
apply on timer-based counter. The 2 seconds update period is the half
of the maximum counting period (4s) of the time-based counter under
period counting mode of the hardware.

Because fully-implemented 64-bit counters do not need this workaround,
enable this hrtimer on the platforms known to have narrowed counter.

Before this patch, when counting fio for 10m the counts is incorrect:
  root@localhost:/tmp# perf stat -e dwc_rootport_20000/rx_pcie_tlp_data_payload/ -- fio --runtime=10m fio_job.config
  [...]
  Run status group 0 (all jobs):
     READ: bw=5594MiB/s (5865MB/s), 5594MiB/s-5594MiB/s (5865MB/s-5865MB/s), io=3278GiB (3519GB), run=600010-600010msec
  [...]
  Performance counter stats for 'system wide':
     137,438,953,456      dwc_rootport_20000/rx_pcie_tlp_data_payload/

After this patch the counts is as expected:
  root@localhost:/tmp# perf stat -e dwc_rootport_20000/rx_pcie_tlp_data_payload/ -- fio --runtime=10m fio_job.config
  [...]
  Run status group 0 (all jobs):
     READ: bw=5632MiB/s (5905MB/s), 5632MiB/s-5632MiB/s (5905MB/s-5905MB/s), io=3300GiB (3543GB), run=600013-600013msec
  [...]
  Performance counter stats for 'system wide':
   3,543,850,268,576      dwc_rootport_20000/rx_pcie_tlp_data_payload/

Signed-off-by: Yufan Dou <douyufan@picoheart.com>
Signed-off-by: Yicong Yang <yang.yicong@picoheart.com>
---
 drivers/perf/dwc_pcie_pmu.c | 71 ++++++++++++++++++++++++++++++++++---
 1 file changed, 67 insertions(+), 4 deletions(-)

diff --git a/drivers/perf/dwc_pcie_pmu.c b/drivers/perf/dwc_pcie_pmu.c
index 5385401fa9cf..f5f3e5b83f5b 100644
--- a/drivers/perf/dwc_pcie_pmu.c
+++ b/drivers/perf/dwc_pcie_pmu.c
@@ -11,6 +11,7 @@
 #include <linux/cpumask.h>
 #include <linux/device.h>
 #include <linux/errno.h>
+#include <linux/hrtimer.h>
 #include <linux/kernel.h>
 #include <linux/list.h>
 #include <linux/pcie-dwc.h>
@@ -83,6 +84,7 @@ enum dwc_pcie_event_type {
 
 #define DWC_PCIE_LANE_EVENT_MAX_PERIOD		GENMASK_ULL(31, 0)
 #define DWC_PCIE_MAX_PERIOD			GENMASK_ULL(63, 0)
+#define DWC_PCIE_PMU_TIMER_PERIOD_NS		(2 * NSEC_PER_SEC)
 
 struct dwc_pcie_pmu {
 	struct pmu		pmu;
@@ -93,6 +95,8 @@ struct dwc_pcie_pmu {
 	/* Groups #6 and #7 */
 	DECLARE_BITMAP(lane_events, 2 * DWC_PCIE_LANE_MAX_EVENTS_PER_GROUP);
 	struct perf_event	*time_based_event;
+	bool			timer_enable;
+	struct hrtimer		hrtimer;
 
 	struct hlist_node	cpuhp_node;
 	int			on_cpu;
@@ -354,6 +358,26 @@ static u64 dwc_pcie_pmu_read_time_based_counter(struct perf_event *event)
 	return val;
 }
 
+static void dwc_pcie_pmu_reset_time_based_counter(struct perf_event *event)
+{
+	struct dwc_pcie_pmu *pcie_pmu = to_dwc_pcie_pmu(event->pmu);
+	struct hw_perf_event *hwc = &event->hw;
+	u64 prev;
+
+	dwc_pcie_pmu_time_based_event_enable(pcie_pmu, false);
+
+	/*
+	 * The hardware counter is reset to zero when disabled. Synchronize
+	 * prev_count so that the next event_update() computes the correct
+	 * delta against the new counter baseline.
+	 */
+	do {
+		prev = local64_read(&hwc->prev_count);
+	} while (local64_cmpxchg(&hwc->prev_count, prev, 0) != prev);
+
+	dwc_pcie_pmu_time_based_event_enable(pcie_pmu, true);
+}
+
 static void dwc_pcie_pmu_event_update(struct perf_event *event)
 {
 	struct hw_perf_event *hwc = &event->hw;
@@ -429,6 +453,26 @@ static int dwc_pcie_pmu_validate_group(struct perf_event *event)
 	return 0;
 }
 
+static enum hrtimer_restart dwc_pcie_pmu_hrtimer_callback(struct hrtimer *hrtimer)
+{
+	struct dwc_pcie_pmu *pcie_pmu = container_of(hrtimer, struct dwc_pcie_pmu, hrtimer);
+	struct perf_event *event = pcie_pmu->time_based_event;
+	struct hw_perf_event *hwc;
+
+	if (!event)
+		return HRTIMER_NORESTART;
+
+	hwc = &event->hw;
+	if (hwc->state & PERF_HES_STOPPED)
+		return HRTIMER_NORESTART;
+
+	dwc_pcie_pmu_event_update(event);
+	dwc_pcie_pmu_reset_time_based_counter(event);
+	hrtimer_forward_now(hrtimer, ns_to_ktime(DWC_PCIE_PMU_TIMER_PERIOD_NS));
+
+	return HRTIMER_RESTART;
+}
+
 static int dwc_pcie_pmu_event_init(struct perf_event *event)
 {
 	struct dwc_pcie_pmu *pcie_pmu = to_dwc_pcie_pmu(event->pmu);
@@ -478,10 +522,15 @@ static void dwc_pcie_pmu_event_start(struct perf_event *event, int flags)
 	hwc->state = 0;
 	local64_set(&hwc->prev_count, 0);
 
-	if (type == DWC_PCIE_LANE_EVENT)
+	if (type == DWC_PCIE_LANE_EVENT) {
 		dwc_pcie_pmu_lane_event_enable(pcie_pmu, event, true);
-	else if (type == DWC_PCIE_TIME_BASE_EVENT)
+	} else if (type == DWC_PCIE_TIME_BASE_EVENT) {
 		dwc_pcie_pmu_time_based_event_enable(pcie_pmu, true);
+		if (pcie_pmu->timer_enable)
+			hrtimer_start(&pcie_pmu->hrtimer,
+				      ns_to_ktime(DWC_PCIE_PMU_TIMER_PERIOD_NS),
+				      HRTIMER_MODE_REL_PINNED_HARD);
+	}
 }
 
 static void dwc_pcie_pmu_event_stop(struct perf_event *event, int flags)
@@ -495,11 +544,15 @@ static void dwc_pcie_pmu_event_stop(struct perf_event *event, int flags)
 
 	dwc_pcie_pmu_event_update(event);
 
-	if (type == DWC_PCIE_LANE_EVENT)
+	if (type == DWC_PCIE_LANE_EVENT) {
 		dwc_pcie_pmu_lane_event_enable(pcie_pmu, event, false);
-	else if (type == DWC_PCIE_TIME_BASE_EVENT)
+	} else if (type == DWC_PCIE_TIME_BASE_EVENT) {
 		dwc_pcie_pmu_time_based_event_enable(pcie_pmu, false);
 
+		if (pcie_pmu->timer_enable)
+			hrtimer_cancel(&pcie_pmu->hrtimer);
+	}
+
 	hwc->state |= PERF_HES_STOPPED | PERF_HES_UPTODATE;
 }
 
@@ -726,6 +779,16 @@ static int dwc_pcie_pmu_probe(struct platform_device *plat_dev)
 	pcie_pmu->ras_des_offset = vsec;
 	pcie_pmu->nr_lanes = pcie_get_width_cap(pdev);
 	pcie_pmu->on_cpu = -1;
+	hrtimer_setup(&pcie_pmu->hrtimer, dwc_pcie_pmu_hrtimer_callback,
+		      CLOCK_MONOTONIC, HRTIMER_MODE_REL_PINNED_HARD);
+
+	/*
+	 * Use timer for updating time-based counts on platforms known
+	 * to have narrowed counter.
+	 */
+	if (pdev->vendor == PCI_VENDOR_ID_PICOHEART)
+		pcie_pmu->timer_enable = true;
+
 	pcie_pmu->pmu = (struct pmu){
 		.name		= name,
 		.parent		= &plat_dev->dev,
-- 
2.50.1 (Apple Git-155)

Re: [PATCH v3 2/4] perf/dwc_pcie: Support narrowed time-based counter for long time monitoring

[sashiko-bot]

> From: Yufan Dou <douyufan@picoheart.com>
> 
> The DWC PCIe Time-Based Analysis Data Register (the counter for time-based
> events) is architected as 64-bit, but some hardware implementations do not
> implement the full width.  On these implementations the counter stops after
> reaching its implemented width. This will limit the usage for short time
> monitoring only. The counter will only cover ~15s for monitoring RX TLP
> payloads on our platform.
> 
> Add an optional hrtimer that fires every 2 seconds. It'll take the role
> as the counter overflow interrupt to read-update-reset the counter and
> event counts to break the limits of the narrow counters. It'll only
> apply on timer-based counter. The 2 seconds update period is the half
> of the maximum counting period (4s) of the time-based counter under
> period counting mode of the hardware.
> [ ... ]
>   Performance counter stats for 'system wide':
>    3,543,850,268,576      dwc_rootport_20000/rx_pcie_tlp_data_payload/
> 
> Signed-off-by: Yufan Dou <douyufan@picoheart.com>
> Signed-off-by: Yicong Yang <yang.yicong@picoheart.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260708084507.98242-1-yang.yicong@picoheart.com?part=2

[PATCH v3 3/4] perf/dwc_pcie: Convert to faux device interface

[Yicong Yang]

The DWC PCIe PMU makes use of the platform device interface but
is not the real device, it's actually the RAS DES capability of
the root port. It's more appropriate to use the lightweight
faux framework to abstract this, it'll be more simple and no
need for the complete device and drivers model. So convert to
the faux device interface.

Move the cpuhp state registration prior to faux device creation
since the probe depends on this and will run by the faux device
creation.

No functional changes intended.

Reviewed-by: Jonathan Cameron <jic23@kernel.org>
Reviewed-by: Shuai Xue <xueshuai@linux.alibaba.com>
Signed-off-by: Yicong Yang <yang.yicong@picoheart.com>
---
 drivers/perf/dwc_pcie_pmu.c | 103 +++++++++++++++++-------------------
 1 file changed, 49 insertions(+), 54 deletions(-)

diff --git a/drivers/perf/dwc_pcie_pmu.c b/drivers/perf/dwc_pcie_pmu.c
index f5f3e5b83f5b..7b4ee71b966b 100644
--- a/drivers/perf/dwc_pcie_pmu.c
+++ b/drivers/perf/dwc_pcie_pmu.c
@@ -10,6 +10,7 @@
 #include <linux/cpuhotplug.h>
 #include <linux/cpumask.h>
 #include <linux/device.h>
+#include <linux/device/faux.h>
 #include <linux/errno.h>
 #include <linux/hrtimer.h>
 #include <linux/kernel.h>
@@ -17,7 +18,6 @@
 #include <linux/pcie-dwc.h>
 #include <linux/perf_event.h>
 #include <linux/pci.h>
-#include <linux/platform_device.h>
 #include <linux/smp.h>
 #include <linux/sysfs.h>
 #include <linux/types.h>
@@ -110,9 +110,10 @@ static struct list_head dwc_pcie_dev_info_head =
 static bool notify;
 
 struct dwc_pcie_dev_info {
-	struct platform_device *plat_dev;
+	struct faux_device *fdev;
 	struct pci_dev *pdev;
 	struct list_head dev_node;
+	char *name;
 };
 
 static ssize_t cpumask_show(struct device *dev,
@@ -660,6 +661,12 @@ static void dwc_pcie_unregister_pmu(void *data)
 	perf_pmu_unregister(&pcie_pmu->pmu);
 }
 
+static int dwc_pcie_pmu_probe(struct faux_device *fdev);
+
+static struct faux_device_ops dwc_pcie_faux_ops = {
+	.probe = dwc_pcie_pmu_probe,
+};
+
 static u16 dwc_pcie_des_cap(struct pci_dev *pdev)
 {
 	const struct dwc_pcie_vsec_id *vid;
@@ -686,31 +693,41 @@ static u16 dwc_pcie_des_cap(struct pci_dev *pdev)
 
 static void dwc_pcie_unregister_dev(struct dwc_pcie_dev_info *dev_info)
 {
-	platform_device_unregister(dev_info->plat_dev);
+	faux_device_destroy(dev_info->fdev);
 	list_del(&dev_info->dev_node);
+	kfree(dev_info->name);
 	kfree(dev_info);
 }
 
 static int dwc_pcie_register_dev(struct pci_dev *pdev)
 {
-	struct platform_device *plat_dev;
 	struct dwc_pcie_dev_info *dev_info;
+	struct faux_device *fdev;
+	char *name;
 	u32 sbdf;
 
 	sbdf = (pci_domain_nr(pdev->bus) << 16) | PCI_DEVID(pdev->bus->number, pdev->devfn);
-	plat_dev = platform_device_register_simple("dwc_pcie_pmu", sbdf, NULL, 0);
-	if (IS_ERR(plat_dev))
-		return PTR_ERR(plat_dev);
+	name = kasprintf(GFP_KERNEL, "dwc_pcie_pmu_%x", sbdf);
+	if (!name)
+		return -ENOMEM;
+
+	fdev = faux_device_create(name, &pdev->dev, &dwc_pcie_faux_ops);
+	if (!fdev) {
+		kfree(name);
+		return -ENODEV;
+	}
 
 	dev_info = kzalloc_obj(*dev_info);
 	if (!dev_info) {
-		platform_device_unregister(plat_dev);
+		faux_device_destroy(fdev);
+		kfree(name);
 		return -ENOMEM;
 	}
 
-	/* Cache platform device to handle pci device hotplug */
-	dev_info->plat_dev = plat_dev;
+	/* Cache faux device to handle pci device hotplug */
+	dev_info->fdev = fdev;
 	dev_info->pdev = pdev;
+	dev_info->name = name;
 	list_add(&dev_info->dev_node, &dwc_pcie_dev_info_head);
 
 	return 0;
@@ -745,33 +762,25 @@ static struct notifier_block dwc_pcie_pmu_nb = {
 	.notifier_call = dwc_pcie_pmu_notifier,
 };
 
-static int dwc_pcie_pmu_probe(struct platform_device *plat_dev)
+static int dwc_pcie_pmu_probe(struct faux_device *fdev)
 {
-	struct pci_dev *pdev;
+	struct pci_dev *pdev = to_pci_dev(fdev->dev.parent);
 	struct dwc_pcie_pmu *pcie_pmu;
 	char *name;
 	u32 sbdf;
 	u16 vsec;
 	int ret;
 
-	sbdf = plat_dev->id;
-	pdev = pci_get_domain_bus_and_slot(sbdf >> 16, PCI_BUS_NUM(sbdf & 0xffff),
-					   sbdf & 0xff);
-	if (!pdev) {
-		pr_err("No pdev found for the sbdf 0x%x\n", sbdf);
-		return -ENODEV;
-	}
+	sbdf = (pci_domain_nr(pdev->bus) << 16) | PCI_DEVID(pdev->bus->number, pdev->devfn);
+	name = devm_kasprintf(&fdev->dev, GFP_KERNEL, "dwc_rootport_%x", sbdf);
+	if (!name)
+		return -ENOMEM;
 
 	vsec = dwc_pcie_des_cap(pdev);
 	if (!vsec)
 		return -ENODEV;
 
-	pci_dev_put(pdev);
-	name = devm_kasprintf(&plat_dev->dev, GFP_KERNEL, "dwc_rootport_%x", sbdf);
-	if (!name)
-		return -ENOMEM;
-
-	pcie_pmu = devm_kzalloc(&plat_dev->dev, sizeof(*pcie_pmu), GFP_KERNEL);
+	pcie_pmu = devm_kzalloc(&fdev->dev, sizeof(*pcie_pmu), GFP_KERNEL);
 	if (!pcie_pmu)
 		return -ENOMEM;
 
@@ -791,7 +800,7 @@ static int dwc_pcie_pmu_probe(struct platform_device *plat_dev)
 
 	pcie_pmu->pmu = (struct pmu){
 		.name		= name,
-		.parent		= &plat_dev->dev,
+		.parent		= &fdev->dev,
 		.module		= THIS_MODULE,
 		.attr_groups	= dwc_pcie_attr_groups,
 		.capabilities	= PERF_PMU_CAP_NO_EXCLUDE,
@@ -812,8 +821,8 @@ static int dwc_pcie_pmu_probe(struct platform_device *plat_dev)
 		return ret;
 	}
 
-	/* Unwind when platform driver removes */
-	ret = devm_add_action_or_reset(&plat_dev->dev,
+	/* Unwind when faux device removes */
+	ret = devm_add_action_or_reset(&fdev->dev,
 				       dwc_pcie_pmu_remove_cpuhp_instance,
 				       &pcie_pmu->cpuhp_node);
 	if (ret)
@@ -824,7 +833,7 @@ static int dwc_pcie_pmu_probe(struct platform_device *plat_dev)
 		pci_err(pdev, "Error %d registering PMU @%x\n", ret, sbdf);
 		return ret;
 	}
-	ret = devm_add_action_or_reset(&plat_dev->dev, dwc_pcie_unregister_pmu,
+	ret = devm_add_action_or_reset(&fdev->dev, dwc_pcie_unregister_pmu,
 				       pcie_pmu);
 	if (ret)
 		return ret;
@@ -876,11 +885,6 @@ static int dwc_pcie_pmu_offline_cpu(unsigned int cpu, struct hlist_node *cpuhp_n
 	return 0;
 }
 
-static struct platform_driver dwc_pcie_pmu_driver = {
-	.probe = dwc_pcie_pmu_probe,
-	.driver = {.name = "dwc_pcie_pmu",},
-};
-
 static void dwc_pcie_cleanup_devices(void)
 {
 	struct dwc_pcie_dev_info *dev_info, *tmp;
@@ -895,6 +899,15 @@ static int __init dwc_pcie_pmu_init(void)
 	struct pci_dev *pdev = NULL;
 	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;
+
 	for_each_pci_dev(pdev) {
 		if (!dwc_pcie_des_cap(pdev))
 			continue;
@@ -906,32 +919,15 @@ static int __init dwc_pcie_pmu_init(void)
 		}
 	}
 
-	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)
-		goto err_cleanup;
-
-	dwc_pcie_pmu_hp_state = ret;
-
-	ret = platform_driver_register(&dwc_pcie_pmu_driver);
-	if (ret)
-		goto err_remove_cpuhp;
-
 	ret = bus_register_notifier(&pci_bus_type, &dwc_pcie_pmu_nb);
 	if (ret)
-		goto err_unregister_driver;
+		goto err_cleanup;
 	notify = true;
 
 	return 0;
-
-err_unregister_driver:
-	platform_driver_unregister(&dwc_pcie_pmu_driver);
-err_remove_cpuhp:
-	cpuhp_remove_multi_state(dwc_pcie_pmu_hp_state);
 err_cleanup:
 	dwc_pcie_cleanup_devices();
+	cpuhp_remove_multi_state(dwc_pcie_pmu_hp_state);
 	return ret;
 }
 
@@ -940,7 +936,6 @@ static void __exit dwc_pcie_pmu_exit(void)
 	if (notify)
 		bus_unregister_notifier(&pci_bus_type, &dwc_pcie_pmu_nb);
 	dwc_pcie_cleanup_devices();
-	platform_driver_unregister(&dwc_pcie_pmu_driver);
 	cpuhp_remove_multi_state(dwc_pcie_pmu_hp_state);
 }
 
-- 
2.50.1 (Apple Git-155)

Re: [PATCH v3 3/4] perf/dwc_pcie: Convert to faux device interface

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

Pre-existing issues:
- [High] Unprotected concurrent access and modification of the global list `dwc_pcie_dev_info_head`, leading to list corruption during PCI device hotplug.
- [High] Missing PM Runtime Management before PCIe config space accesses.
- [Medium] TOCTOU window between iterating existing devices and registering the bus notifier allows missed hotplug events.
--

--- Patch [3]: [PATCH v3 3/4] perf/dwc_pcie: Convert to faux device interface ---
commit f158e1644b202fefbbe3ae998c84159858150d0e
Author: Yicong Yang <yang.yicong@picoheart.com>

perf/dwc_pcie: Convert to faux device interface

This patch converts the DWC PCIe PMU from using the platform device
interface to the lightweight faux device framework. It also moves the
cpuhp state registration prior to faux device creation.

> diff --git a/drivers/perf/dwc_pcie_pmu.c b/drivers/perf/dwc_pcie_pmu.c
> index f5f3e5b83f5bc..7b4ee71b966b0 100644
> --- a/drivers/perf/dwc_pcie_pmu.c
> +++ b/drivers/perf/dwc_pcie_pmu.c

[ ... ]

> @@ -686,31 +693,41 @@ static u16 dwc_pcie_des_cap(struct pci_dev *pdev)
>  
>  static void dwc_pcie_unregister_dev(struct dwc_pcie_dev_info *dev_info)
>  {
> -	platform_device_unregister(dev_info->plat_dev);
> +	faux_device_destroy(dev_info->fdev);
>  	list_del(&dev_info->dev_node);

[Severity: High]
This is a pre-existing issue, but is it safe to modify dwc_pcie_dev_info_head
without a lock here?

Since dwc_pcie_pmu_notifier() executes concurrently for different root ports
on PCI hotplug events, modifying this global list via list_del() and list_add()
without synchronization primitives could lead to list corruption and panics.

> +	kfree(dev_info->name);
>  	kfree(dev_info);
>  }

[ ... ]

> -static int dwc_pcie_pmu_probe(struct platform_device *plat_dev)
> +static int dwc_pcie_pmu_probe(struct faux_device *fdev)
>  {
> -	struct pci_dev *pdev;
> +	struct pci_dev *pdev = to_pci_dev(fdev->dev.parent);

[ ... ]

>  	vsec = dwc_pcie_des_cap(pdev);
>  	if (!vsec)
>  		return -ENODEV;

[Severity: High]
This isn't a bug introduced by this patch, but does the hardware access in
dwc_pcie_des_cap() and other event callbacks have sufficient PM runtime
protection?

If the device is dynamically suspended (D3hot), accessing the configuration
space without holding a PM runtime reference could return corrupted data
or trigger an asynchronous bus fault.

[ ... ]

> @@ -895,6 +899,15 @@ static int __init dwc_pcie_pmu_init(void)
>  	struct pci_dev *pdev = NULL;
>  	int ret;

[ ... ]

> -	ret = bus_register_notifier(&pci_bus_type, &dwc_pcie_pmu_nb);
> -	if (ret)
> -		goto err_unregister_driver;
> +	ret = bus_register_notifier(&pci_bus_type, &dwc_pcie_pmu_nb);
> +	if (ret)
> +		goto err_cleanup;
>  	notify = true;

[Severity: Medium]
This is a pre-existing issue, but could this sequence miss hotplug events?

By iterating existing PCI devices before registering the bus notifier,
any device added or removed in the tiny window between the loop's end
and the notifier activation could be missed.

If a removal is missed, it would permanently leak the PMU structure and
the pci_dev reference.

>  
>  	return 0;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260708084507.98242-1-yang.yicong@picoheart.com?part=3

[PATCH v3 4/4] drivers/perf: Fix the condition where the root port is runtime suspended

[Yicong Yang]

The PMU is based on the corresponding root port's VSEC capability,
which could be inaccessible if the target root port is runtime
suspended to D3Cold. Fix this by getting/putting the root port's
runtime pm when the event of this PMU is init/destroy. It cannot
be done in the add()/del() callbacks where the events is actually
scheduled, since there's not way to runtime resume the device
synchronously in the atomic context (add()/del()).

With this patch, the root port will keep D0 while counting and suspend
after counting finished (for prove the correctness since my board does
not support D3Cold):
  root@localhost:~# cat /sys/bus/pci/devices/0008\:00\:00.0/power_state
  D3hot
  root@localhost:~# perf stat -e dwc_rootport_8000/one_cycle/ &
  [1] 14580
  root@localhost:~# cat /sys/bus/pci/devices/0008\:00\:00.0/power_state
  D0
  root@localhost:~# kill %1
  [1]+  Terminated              perf stat -e dwc_rootport_8000/one_cycle/
  root@localhost:~# cat /sys/bus/pci/devices/0008\:00\:00.0/power_state
  D3hot

The root port also needs to be runtime resumed during PCIe PMU
device registration and probe, so get/put the runtime pm there
as well.

Fixes: af9597adc2f1 ("drivers/perf: add DesignWare PCIe PMU driver")
Closes: https://lore.kernel.org/linux-pci/20260629094234.F25E71F000E9@smtp.kernel.org/
Reported-by: Sashiko <sashiko-bot@kernel.org>
Signed-off-by: Yicong Yang <yang.yicong@picoheart.com>
---
 drivers/perf/dwc_pcie_pmu.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/drivers/perf/dwc_pcie_pmu.c b/drivers/perf/dwc_pcie_pmu.c
index 7b4ee71b966b..aea387d0d0fe 100644
--- a/drivers/perf/dwc_pcie_pmu.c
+++ b/drivers/perf/dwc_pcie_pmu.c
@@ -17,6 +17,7 @@
 #include <linux/list.h>
 #include <linux/pcie-dwc.h>
 #include <linux/perf_event.h>
+#include <linux/pm_runtime.h>
 #include <linux/pci.h>
 #include <linux/smp.h>
 #include <linux/sysfs.h>
@@ -474,12 +475,20 @@ static enum hrtimer_restart dwc_pcie_pmu_hrtimer_callback(struct hrtimer *hrtime
 	return HRTIMER_RESTART;
 }
 
+static void dwc_pcie_pmu_event_destroy(struct perf_event *event)
+{
+	struct dwc_pcie_pmu *pcie_pmu = to_dwc_pcie_pmu(event->pmu);
+
+	pm_runtime_put_autosuspend(&pcie_pmu->pdev->dev);
+}
+
 static int dwc_pcie_pmu_event_init(struct perf_event *event)
 {
 	struct dwc_pcie_pmu *pcie_pmu = to_dwc_pcie_pmu(event->pmu);
 	enum dwc_pcie_event_type type = DWC_PCIE_EVENT_TYPE(event);
 	struct perf_event *sibling;
 	u32 lane;
+	int ret;
 
 	if (event->attr.type != event->pmu->type)
 		return -ENOENT;
@@ -509,7 +518,16 @@ static int dwc_pcie_pmu_event_init(struct perf_event *event)
 	if (dwc_pcie_pmu_validate_group(event))
 		return -ENOSPC;
 
+	/*
+	 * Resume the root port for VSEC access. Use RPM_TRANSPARENT as the
+	 * VSEC is accessible if root port's runtime pm is disabled.
+	 */
+	ret = pm_runtime_get_active(&pcie_pmu->pdev->dev, RPM_TRANSPARENT);
+	if (ret < 0)
+		return ret;
+
 	event->cpu = pcie_pmu->on_cpu;
+	event->destroy = dwc_pcie_pmu_event_destroy;
 
 	return 0;
 }
@@ -776,6 +794,7 @@ static int dwc_pcie_pmu_probe(struct faux_device *fdev)
 	if (!name)
 		return -ENOMEM;
 
+	guard(pm_runtime_active_auto)(&pdev->dev);
 	vsec = dwc_pcie_des_cap(pdev);
 	if (!vsec)
 		return -ENODEV;
@@ -909,6 +928,8 @@ static int __init dwc_pcie_pmu_init(void)
 	dwc_pcie_pmu_hp_state = ret;
 
 	for_each_pci_dev(pdev) {
+		guard(pm_runtime_active_auto)(&pdev->dev);
+
 		if (!dwc_pcie_des_cap(pdev))
 			continue;
 
-- 
2.50.1 (Apple Git-155)

Re: [PATCH v3 4/4] drivers/perf: Fix the condition where the root port is runtime suspended

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Ignored return value of runtime PM resume leading to potential hardware access while in D3cold
- [High] System-wide PCI device wake-up during module initialization
- [High] Use-After-Free of PCI device structure due to mixing goto with guard() cleanup
--

--- Patch [4]: [PATCH v3 4/4] drivers/perf: Fix the condition where the root port is runtime suspended ---
commit 366af2eb4d6dbafe5fed360f0136d4ac3c45f174
Author: Yicong Yang <yang.yicong@picoheart.com>

drivers/perf: Fix the condition where the root port is runtime suspended

The patch adds runtime PM get/put when initializing and destroying PMU
events to ensure the PCIe root port is not suspended during VSEC capability
access. It also adds runtime PM guards during probe and system-wide
initialization.

Link: https://lore.kernel.org/linux-pci/20260629094234.F25E71F000E9@smtp.kernel.org/

> diff --git a/drivers/perf/dwc_pcie_pmu.c b/drivers/perf/dwc_pcie_pmu.c
> index 7b4ee71b966b0..aea387d0d0fe3 100644
> --- a/drivers/perf/dwc_pcie_pmu.c
> +++ b/drivers/perf/dwc_pcie_pmu.c

[ ... ]

> @@ -776,6 +794,7 @@ static int dwc_pcie_pmu_probe(struct faux_device *fdev)
>  	if (!name)
>  		return -ENOMEM;
>  
> +	guard(pm_runtime_active_auto)(&pdev->dev);

[Severity: High]
The guard macro will resume the device, but its return value is ignored.

If the device fails to resume, could we end up reading the PCI
configuration space of a suspended device in dwc_pcie_des_cap() right
after this?

>  	vsec = dwc_pcie_des_cap(pdev);
>  	if (!vsec)
>  		return -ENODEV;

[ ... ]

> @@ -909,6 +928,8 @@ static int __init dwc_pcie_pmu_init(void)
>  	dwc_pcie_pmu_hp_state = ret;
>  
>  	for_each_pci_dev(pdev) {
> +		guard(pm_runtime_active_auto)(&pdev->dev);
> +

[Severity: High]
Because this loop iterates over every PCI device in the system, does this
guard force every suspended PCI device to synchronously wake up before
we can check if it is actually the target device type?

Does this also share the same ignored return value issue as
dwc_pcie_pmu_probe(), where a failed resume goes unchecked?

Additionally, could this guard cause a use-after-free on the error path
later in the loop?

In dwc_pcie_pmu_init(), if dwc_pcie_register_dev() fails, we drop the
device reference and jump out of the block:

	ret = dwc_pcie_register_dev(pdev);
	if (ret) {
		pci_dev_put(pdev);
		goto err_cleanup;
	}

Since goto jumps out of the loop block, the compiler runs the cleanup
function for the guard (pm_runtime_put_autosuspend) after the manual
pci_dev_put() call.

If the manual put drops the last reference, does the cleanup function
dereference a freed pointer?

>  		if (!dwc_pcie_des_cap(pdev))
>  			continue;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260708084507.98242-1-yang.yicong@picoheart.com?part=4