[PATCH v1 0/3] New vendor support and optimizations for DWC PCIe PMU

[PATCH v1 0/3] New vendor support and optimizations for DWC PCIe PMU

[Yicong Yang]

This patchset mainly enable the DWC PCIe PMU on our platform (Patch 1/3).
Besides do some optimization and refactor:
- Patch 2/3 extend the monitoring limit on platforms with narrowed counter width (< 64 bit)
- Patch 3/3 switch the driver to use the faux device interface

Yicong Yang (2):
  perf/dwc_pcie: Add support for Picoheart vendor devices
  perf/dwc_pcie: Convert to faux device interface

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

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

-- 
2.50.1 (Apple Git-155)

[PATCH v1 1/3] 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 vendorid to enable the PCIe
PMU support.

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 24cb42f66e4b..7758cf341aa9 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..b0ccb2ad74db 100644
--- a/include/linux/pcie-dwc.h
+++ b/include/linux/pcie-dwc.h
@@ -32,6 +32,8 @@ static const struct dwc_pcie_vsec_id dwc_pcie_rasdes_vsec_ids[] = {
 	  .vsec_id = 0x02, .vsec_rev = 0x4 },
 	{ .vendor_id = PCI_VENDOR_ID_SAMSUNG,
 	  .vsec_id = 0x02, .vsec_rev = 0x4 },
+	{ .vendor_id = PCI_VENDOR_ID_PICOHEART,
+	  .vsec_id = 0x02, .vsec_rev = 0x4 },
 	{}
 };
 
-- 
2.50.1 (Apple Git-155)

[PATCH v1 2/3] 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,
expose it via the module parameter timer_enable (default N) so that
users can opt in only on affected platforms.

Before this patch, when counting fio for 10m the counts is incorrect:
  root@localhost:/tmp# echo 0 > /sys/module/dwc_pcie_pmu/parameters/timer_enable
  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# echo 1 > /sys/module/dwc_pcie_pmu/parameters/timer_enable
  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 | 59 +++++++++++++++++++++++++++++++++++--
 1 file changed, 57 insertions(+), 2 deletions(-)

diff --git a/drivers/perf/dwc_pcie_pmu.c b/drivers/perf/dwc_pcie_pmu.c
index 5385401fa9cf..abf50f173202 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,11 @@ 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)
+
+static bool timer_enable;
+module_param_named(timer_enable, timer_enable, bool, 0644);
+MODULE_PARM_DESC(timer_enable, "Enable hrtimer for time-based events (default: N)");
 
 struct dwc_pcie_pmu {
 	struct pmu		pmu;
@@ -93,6 +99,7 @@ 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;
+	struct hrtimer		hrtimer;
 
 	struct hlist_node	cpuhp_node;
 	int			on_cpu;
@@ -354,6 +361,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 +456,25 @@ 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);
@@ -480,8 +526,13 @@ static void dwc_pcie_pmu_event_start(struct perf_event *event, int flags)
 
 	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 (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)
@@ -497,8 +548,10 @@ static void dwc_pcie_pmu_event_stop(struct perf_event *event, int flags)
 
 	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);
+		hrtimer_cancel(&pcie_pmu->hrtimer);
+	}
 
 	hwc->state |= PERF_HES_STOPPED | PERF_HES_UPTODATE;
 }
@@ -726,6 +779,8 @@ 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);
 	pcie_pmu->pmu = (struct pmu){
 		.name		= name,
 		.parent		= &plat_dev->dev,
-- 
2.50.1 (Apple Git-155)

[PATCH v1 3/3] 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.

No functional changes intended.

Signed-off-by: Yicong Yang <yang.yicong@picoheart.com>
---
 drivers/perf/dwc_pcie_pmu.c | 101 +++++++++++++++++-------------------
 1 file changed, 48 insertions(+), 53 deletions(-)

diff --git a/drivers/perf/dwc_pcie_pmu.c b/drivers/perf/dwc_pcie_pmu.c
index abf50f173202..fee040f8b833 100644
--- a/drivers/perf/dwc_pcie_pmu.c
+++ b/drivers/perf/dwc_pcie_pmu.c
@@ -17,10 +17,10 @@
 #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>
+#include <linux/device/faux.h>
 
 #define DWC_PCIE_EVENT_CNT_CTL			0x8
 
@@ -113,9 +113,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;
 
@@ -783,7 +792,7 @@ static int dwc_pcie_pmu_probe(struct platform_device *plat_dev)
 		      CLOCK_MONOTONIC, HRTIMER_MODE_REL_PINNED_HARD);
 	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,
@@ -805,7 +814,7 @@ static int dwc_pcie_pmu_probe(struct platform_device *plat_dev)
 	}
 
 	/* Unwind when platform driver removes */
-	ret = devm_add_action_or_reset(&plat_dev->dev,
+	ret = devm_add_action_or_reset(&fdev->dev,
 				       dwc_pcie_pmu_remove_cpuhp_instance,
 				       &pcie_pmu->cpuhp_node);
 	if (ret)
@@ -816,7 +825,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;
@@ -868,11 +877,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;
@@ -887,6 +891,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;
@@ -898,32 +911,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;
 }
 
@@ -932,7 +928,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 v1 2/3] perf/dwc_pcie: Support narrowed time-based counter for long time monitoring

[Jonathan Cameron]

On Mon, 15 Jun 2026 14:34:58 +0800
"Yicong Yang" <yang.yicong@picoheart.com> wrote:

> 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,
> expose it via the module parameter timer_enable (default N) so that
> users can opt in only on affected platforms.
Hi,

That bit seems less than ideal. Given there is matching by the vsec
in the previous patch, can we make this automatically opt in on appropriate
platforms?

That would require the vendor to not use the same vsec for devices
that don't have this quirk so kind of depends what is already out there.

If this is a 'feature' rather than a 'bug' then longer term it would ideally
be discoverable from a capability style register.

A few other minor things inline,

Thanks,

Jonathan


> 
> Before this patch, when counting fio for 10m the counts is incorrect:
>   root@localhost:/tmp# echo 0 > /sys/module/dwc_pcie_pmu/parameters/timer_enable
>   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# echo 1 > /sys/module/dwc_pcie_pmu/parameters/timer_enable
>   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>

> +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));

Small readability thing but I would put a blank line here.

> +	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);
> @@ -480,8 +526,13 @@ static void dwc_pcie_pmu_event_start(struct perf_event *event, int flags)
>  
>  	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) {
Same as below - if () {
		} else if {
		}
>  		dwc_pcie_pmu_time_based_event_enable(pcie_pmu, true);
> +		if (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)
> @@ -497,8 +548,10 @@ static void dwc_pcie_pmu_event_stop(struct perf_event *event, int flags)
>  
>  	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) {

Style wise, all legs should have {} if any need it, so add them to the if above as well.

>  		dwc_pcie_pmu_time_based_event_enable(pcie_pmu, false);
> +		hrtimer_cancel(&pcie_pmu->hrtimer);
> +	}
>  
>  	hwc->state |= PERF_HES_STOPPED | PERF_HES_UPTODATE;
>  }
> @@ -726,6 +779,8 @@ 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);
>  	pcie_pmu->pmu = (struct pmu){
>  		.name		= name,
>  		.parent		= &plat_dev->dev,

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

[Jonathan Cameron]

On Mon, 15 Jun 2026 14:34:59 +0800
"Yicong Yang" <yang.yicong@picoheart.com> wrote:

> 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.
I'd perhaps add a little here to say that he cpu hp callbacks might
be reordered wrt to the registration.  That looks fine to me but
it's a little more complex than this patch description made me think.

> 
> No functional changes intended.
> 
> Signed-off-by: Yicong Yang <yang.yicong@picoheart.com>
> ---
>  drivers/perf/dwc_pcie_pmu.c | 101 +++++++++++++++++-------------------
>  1 file changed, 48 insertions(+), 53 deletions(-)
> 
> diff --git a/drivers/perf/dwc_pcie_pmu.c b/drivers/perf/dwc_pcie_pmu.c
> index abf50f173202..fee040f8b833 100644
> --- a/drivers/perf/dwc_pcie_pmu.c
> +++ b/drivers/perf/dwc_pcie_pmu.c
> @@ -17,10 +17,10 @@
>  #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>
> +#include <linux/device/faux.h>
>  
Why there?  Seems like they are alphabetical order so should be up
under d.

Other than that looks fine to me.

Reviewed-by: Jonathan Cameron <jic23@kernel.org>

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

[Yicong Yang]

On 6/16/26 7:34 PM, Jonathan Cameron wrote:
> On Mon, 15 Jun 2026 14:34:58 +0800
> "Yicong Yang" <yang.yicong@picoheart.com> wrote:
> 
>> 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,
>> expose it via the module parameter timer_enable (default N) so that
>> users can opt in only on affected platforms.
> Hi,
> 
> That bit seems less than ideal. Given there is matching by the vsec
> in the previous patch, can we make this automatically opt in on appropriate
> platforms?

sure, we can make this opt in on our platform first by matching the
root port's vendor ID. it's implemented as a module parameter since
no discovery mechanism for this, then user can enable this if they found
they can only do short time monitoring (symptom attached in the commit)

> 
> That would require the vendor to not use the same vsec for devices
> that don't have this quirk so kind of depends what is already out there.
> 
> If this is a 'feature' rather than a 'bug' then longer term it would ideally
> be discoverable from a capability style register.

I may found that it's hard to regard this as a feature or bug, but
the implementation choice of the vendor...

> 
> A few other minor things inline,
> 

will fix in the next version.

Thanks.

> Thanks,
> 
> Jonathan
> 
> 
>>
>> Before this patch, when counting fio for 10m the counts is incorrect:
>>   root@localhost:/tmp# echo 0 > /sys/module/dwc_pcie_pmu/parameters/timer_enable
>>   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# echo 1 > /sys/module/dwc_pcie_pmu/parameters/timer_enable
>>   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>
> 
>> +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));
> 
> Small readability thing but I would put a blank line here.
> 
>> +	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);
>> @@ -480,8 +526,13 @@ static void dwc_pcie_pmu_event_start(struct perf_event *event, int flags)
>>  
>>  	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) {
> Same as below - if () {
> 		} else if {
> 		}
>>  		dwc_pcie_pmu_time_based_event_enable(pcie_pmu, true);
>> +		if (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)
>> @@ -497,8 +548,10 @@ static void dwc_pcie_pmu_event_stop(struct perf_event *event, int flags)
>>  
>>  	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) {
> 
> Style wise, all legs should have {} if any need it, so add them to the if above as well.
> 
>>  		dwc_pcie_pmu_time_based_event_enable(pcie_pmu, false);
>> +		hrtimer_cancel(&pcie_pmu->hrtimer);
>> +	}
>>  
>>  	hwc->state |= PERF_HES_STOPPED | PERF_HES_UPTODATE;
>>  }
>> @@ -726,6 +779,8 @@ 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);
>>  	pcie_pmu->pmu = (struct pmu){
>>  		.name		= name,
>>  		.parent		= &plat_dev->dev,

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

[Yicong Yang]

On 6/16/26 8:53 PM, Jonathan Cameron wrote:
> On Mon, 15 Jun 2026 14:34:59 +0800
> "Yicong Yang" <yang.yicong@picoheart.com> wrote:
> 
>> 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.
> I'd perhaps add a little here to say that he cpu hp callbacks might
> be reordered wrt to the registration.  That looks fine to me but
> it's a little more complex than this patch description made me think.
> 

sure. will mention the reason in the commit.

the original code registered them in the order:
  platform_devices->cpuhp_callback->platform_driver

it would be more reasonable to register the cpuhp callback at first
but it works since the drv::probe is called after the cpuhp callback
is registered. with faux we need to move the cpuhp at first before
faux device's registration will invoke the probe subsequently which
depends on the cpuhp callback.

>>
>> No functional changes intended.
>>
>> Signed-off-by: Yicong Yang <yang.yicong@picoheart.com>
>> ---
>>  drivers/perf/dwc_pcie_pmu.c | 101 +++++++++++++++++-------------------
>>  1 file changed, 48 insertions(+), 53 deletions(-)
>>
>> diff --git a/drivers/perf/dwc_pcie_pmu.c b/drivers/perf/dwc_pcie_pmu.c
>> index abf50f173202..fee040f8b833 100644
>> --- a/drivers/perf/dwc_pcie_pmu.c
>> +++ b/drivers/perf/dwc_pcie_pmu.c
>> @@ -17,10 +17,10 @@
>>  #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>
>> +#include <linux/device/faux.h>
>>  
> Why there?  Seems like they are alphabetical order so should be up
> under d.

will make them alphabetical regardless of the flat/nested
path :)

> 
> Other than that looks fine to me.
> 
> Reviewed-by: Jonathan Cameron <jic23@kernel.org>

Thanks.