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

[sashiko-bot@kernel.org]

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