[PATCH 0/4] perf/dwc_pcie: Fix registration issue in multi PCIe controller instances

[PATCH 0/4] perf/dwc_pcie: Fix registration issue in multi PCIe controller instances

[Krishna chaitanya chundru]

When there are multiple of instances of PCIe controllers, registration
to perf driver fails with this error. This is because of having same
bdf value for devices under two different controllers.

Update the logic to use sbdf which is a unique number in case of
multi instance also.

When the PCIe devices are discovered late, the driver can't find
the PCIe devices and returns in the init without registering with
the bus notifier. Due to that the devices which are discovered late
the driver can't register for this.

Register for bus notifier even if the device is not found in init.

Update the vendor table with QCOM PCIe vendorid to support QCOM devices.

Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com>
---
Krishna chaitanya chundru (4):
      perf/dwc_pcie: Fix registration issue in multi PCIe controller instances
      Documentation: dwc_pcie_pmu: Update bdf to sbdf
      perf/dwc_pcie: Always register for PCIe bus notifier
      perf/dwc_pcie: Add support for QCOM vendor devices

 Documentation/admin-guide/perf/dwc_pcie_pmu.rst | 16 +++++++--------
 drivers/perf/dwc_pcie_pmu.c                     | 27 +++++++++++++------------
 2 files changed, 22 insertions(+), 21 deletions(-)
---
base-commit: b236787b0da563e3bad0dab1b4b9a5bb54eabd39
change-id: 20240731-dwc_pmu_fix-3729bd3657fe
prerequisite-change-id: 20240728-mhi_runtime_pm-2383b74c71ed:v1

Best regards,
-- 
Krishna chaitanya chundru <quic_krichai@quicinc.com>

[PATCH 1/4] perf/dwc_pcie: Fix registration issue in multi PCIe controller instances

[Krishna chaitanya chundru]

When there are multiple of instances of PCIe controllers, registration
to perf driver fails with this error.
sysfs: cannot create duplicate filename '/devices/platform/dwc_pcie_pmu.0'
CPU: 0 PID: 166 Comm: modprobe Not tainted 6.10.0-rc2-next-20240607-dirty
Hardware name: Qualcomm SA8775P Ride (DT)
Call trace:
 dump_backtrace.part.8+0x98/0xf0
 show_stack+0x14/0x1c
 dump_stack_lvl+0x74/0x88
 dump_stack+0x14/0x1c
 sysfs_warn_dup+0x60/0x78
 sysfs_create_dir_ns+0xe8/0x100
 kobject_add_internal+0x94/0x224
 kobject_add+0xa8/0x118
 device_add+0x298/0x7b4
 platform_device_add+0x1a0/0x228
 platform_device_register_full+0x11c/0x148
 dwc_pcie_register_dev+0x74/0xf0 [dwc_pcie_pmu]
 dwc_pcie_pmu_init+0x7c/0x1000 [dwc_pcie_pmu]
 do_one_initcall+0x58/0x1c0
 do_init_module+0x58/0x208
 load_module+0x1804/0x188c
 __do_sys_init_module+0x18c/0x1f0
 __arm64_sys_init_module+0x14/0x1c
 invoke_syscall+0x40/0xf8
 el0_svc_common.constprop.1+0x70/0xf4
 do_el0_svc+0x18/0x20
 el0_svc+0x28/0xb0
 el0t_64_sync_handler+0x9c/0xc0
 el0t_64_sync+0x160/0x164
kobject: kobject_add_internal failed for dwc_pcie_pmu.0 with -EEXIST,
don't try to register things with the same name in the same directory.

This is because of having same bdf value for devices under two different
controllers.

Update the logic to use sbdf which is a unique number in case of
multi instance also.

Fixes: af9597adc2f1 ("drivers/perf: add DesignWare PCIe PMU driver")
Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com>
---
 drivers/perf/dwc_pcie_pmu.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/perf/dwc_pcie_pmu.c b/drivers/perf/dwc_pcie_pmu.c
index c5e328f23841..c115348b8d53 100644
--- a/drivers/perf/dwc_pcie_pmu.c
+++ b/drivers/perf/dwc_pcie_pmu.c
@@ -556,10 +556,10 @@ static int dwc_pcie_register_dev(struct pci_dev *pdev)
 {
 	struct platform_device *plat_dev;
 	struct dwc_pcie_dev_info *dev_info;
-	u32 bdf;
+	u32 sbdf;
 
-	bdf = PCI_DEVID(pdev->bus->number, pdev->devfn);
-	plat_dev = platform_device_register_data(NULL, "dwc_pcie_pmu", bdf,
+	sbdf = (pci_domain_nr(pdev->bus) << 16) | PCI_DEVID(pdev->bus->number, pdev->devfn);
+	plat_dev = platform_device_register_data(NULL, "dwc_pcie_pmu", sbdf,
 						 pdev, sizeof(*pdev));
 
 	if (IS_ERR(plat_dev))
@@ -611,15 +611,15 @@ static int dwc_pcie_pmu_probe(struct platform_device *plat_dev)
 	struct pci_dev *pdev = plat_dev->dev.platform_data;
 	struct dwc_pcie_pmu *pcie_pmu;
 	char *name;
-	u32 bdf, val;
+	u32 sbdf, val;
 	u16 vsec;
 	int ret;
 
 	vsec = pci_find_vsec_capability(pdev, pdev->vendor,
 					DWC_PCIE_VSEC_RAS_DES_ID);
 	pci_read_config_dword(pdev, vsec + PCI_VNDR_HEADER, &val);
-	bdf = PCI_DEVID(pdev->bus->number, pdev->devfn);
-	name = devm_kasprintf(&plat_dev->dev, GFP_KERNEL, "dwc_rootport_%x", bdf);
+	sbdf = (pci_domain_nr(pdev->bus) << 16) | PCI_DEVID(pdev->bus->number, pdev->devfn);
+	name = devm_kasprintf(&plat_dev->dev, GFP_KERNEL, "dwc_rootport_%x", sbdf);
 	if (!name)
 		return -ENOMEM;
 
@@ -650,7 +650,7 @@ static int dwc_pcie_pmu_probe(struct platform_device *plat_dev)
 	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);
+		pci_err(pdev, "Error %d registering hotplug @%x\n", ret, sbdf);
 		return ret;
 	}
 
@@ -663,7 +663,7 @@ static int dwc_pcie_pmu_probe(struct platform_device *plat_dev)
 
 	ret = perf_pmu_register(&pcie_pmu->pmu, name, -1);
 	if (ret) {
-		pci_err(pdev, "Error %d registering PMU @%x\n", ret, bdf);
+		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,

-- 
2.34.1

[PATCH 2/4] Documentation: dwc_pcie_pmu: Update bdf to sbdf

[Krishna chaitanya chundru]

Update document to reflect the driver change to use sbdf instead
of bdf alone.

Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com>
---
 Documentation/admin-guide/perf/dwc_pcie_pmu.rst | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/Documentation/admin-guide/perf/dwc_pcie_pmu.rst b/Documentation/admin-guide/perf/dwc_pcie_pmu.rst
index d47cd229d710..39b8e1fdd0cd 100644
--- a/Documentation/admin-guide/perf/dwc_pcie_pmu.rst
+++ b/Documentation/admin-guide/perf/dwc_pcie_pmu.rst
@@ -46,16 +46,16 @@ Some of the events only exist for specific configurations.
 DesignWare Cores (DWC) PCIe PMU Driver
 =======================================
 
-This driver adds PMU devices for each PCIe Root Port named based on the BDF of
+This driver adds PMU devices for each PCIe Root Port named based on the SBDF of
 the Root Port. For example,
 
-    30:03.0 PCI bridge: Device 1ded:8000 (rev 01)
+    0001:30:03.0 PCI bridge: Device 1ded:8000 (rev 01)
 
-the PMU device name for this Root Port is dwc_rootport_3018.
+the PMU device name for this Root Port is dwc_rootport_13018.
 
 The DWC PCIe PMU driver registers a perf PMU driver, which provides
 description of available events and configuration options in sysfs, see
-/sys/bus/event_source/devices/dwc_rootport_{bdf}.
+/sys/bus/event_source/devices/dwc_rootport_{sbdf}.
 
 The "format" directory describes format of the config fields of the
 perf_event_attr structure. The "events" directory provides configuration
@@ -66,16 +66,16 @@ The "perf list" command shall list the available events from sysfs, e.g.::
 
     $# perf list | grep dwc_rootport
     <...>
-    dwc_rootport_3018/Rx_PCIe_TLP_Data_Payload/        [Kernel PMU event]
+    dwc_rootport_13018/Rx_PCIe_TLP_Data_Payload/        [Kernel PMU event]
     <...>
-    dwc_rootport_3018/rx_memory_read,lane=?/               [Kernel PMU event]
+    dwc_rootport_13018/rx_memory_read,lane=?/               [Kernel PMU event]
 
 Time Based Analysis Event Usage
 -------------------------------
 
 Example usage of counting PCIe RX TLP data payload (Units of bytes)::
 
-    $# perf stat -a -e dwc_rootport_3018/Rx_PCIe_TLP_Data_Payload/
+    $# perf stat -a -e dwc_rootport_13018/Rx_PCIe_TLP_Data_Payload/
 
 The average RX/TX bandwidth can be calculated using the following formula:
 
@@ -88,7 +88,7 @@ Lane Event Usage
 Each lane has the same event set and to avoid generating a list of hundreds
 of events, the user need to specify the lane ID explicitly, e.g.::
 
-    $# perf stat -a -e dwc_rootport_3018/rx_memory_read,lane=4/
+    $# perf stat -a -e dwc_rootport_13018/rx_memory_read,lane=4/
 
 The driver does not support sampling, therefore "perf record" will not
 work. Per-task (without "-a") perf sessions are not supported.

-- 
2.34.1

[PATCH 3/4] perf/dwc_pcie: Always register for PCIe bus notifier

[Krishna chaitanya chundru]

When the PCIe devices are discovered late, the driver can't find
the PCIe devices and returns in the init without registering with
the bus notifier. Due to that the devices which are discovered late
the driver can't register for this.

Register for bus notifier even if the device is not found in init.

Fixes: af9597adc2f1 ("drivers/perf: add DesignWare PCIe PMU driver")
Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com>
---
 drivers/perf/dwc_pcie_pmu.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/perf/dwc_pcie_pmu.c b/drivers/perf/dwc_pcie_pmu.c
index c115348b8d53..aa1010b44bcb 100644
--- a/drivers/perf/dwc_pcie_pmu.c
+++ b/drivers/perf/dwc_pcie_pmu.c
@@ -741,8 +741,6 @@ static int __init dwc_pcie_pmu_init(void)
 
 		found = true;
 	}
-	if (!found)
-		return -ENODEV;
 
 	ret = cpuhp_setup_state_multi(CPUHP_AP_ONLINE_DYN,
 				      "perf/dwc_pcie_pmu:online",
@@ -753,9 +751,11 @@ static int __init dwc_pcie_pmu_init(void)
 
 	dwc_pcie_pmu_hp_state = ret;
 
-	ret = platform_driver_register(&dwc_pcie_pmu_driver);
-	if (ret)
-		goto platform_driver_register_err;
+	if (!found) {
+		ret = platform_driver_register(&dwc_pcie_pmu_driver);
+		if (ret)
+			goto platform_driver_register_err;
+	}
 
 	ret = bus_register_notifier(&pci_bus_type, &dwc_pcie_pmu_nb);
 	if (ret)

-- 
2.34.1

[PATCH 4/4] perf/dwc_pcie: Add support for QCOM vendor devices

[Krishna chaitanya chundru]

Update the vendor table with QCOM PCIe vendorid.

Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com>
---
 drivers/perf/dwc_pcie_pmu.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/perf/dwc_pcie_pmu.c b/drivers/perf/dwc_pcie_pmu.c
index aa1010b44bcb..ea73ae5c45c5 100644
--- a/drivers/perf/dwc_pcie_pmu.c
+++ b/drivers/perf/dwc_pcie_pmu.c
@@ -107,6 +107,7 @@ struct dwc_pcie_vendor_id {
 
 static const struct dwc_pcie_vendor_id dwc_pcie_vendor_ids[] = {
 	{.vendor_id = PCI_VENDOR_ID_ALIBABA },
+	{.vendor_id = PCI_VENDOR_ID_QCOM },
 	{} /* terminator */
 };
 

-- 
2.34.1

Re: [PATCH 1/4] perf/dwc_pcie: Fix registration issue in multi PCIe controller instances

[Yicong Yang]

On 2024/7/31 12:23, Krishna chaitanya chundru wrote:
> When there are multiple of instances of PCIe controllers, registration
> to perf driver fails with this error.
> sysfs: cannot create duplicate filename '/devices/platform/dwc_pcie_pmu.0'
> CPU: 0 PID: 166 Comm: modprobe Not tainted 6.10.0-rc2-next-20240607-dirty
> Hardware name: Qualcomm SA8775P Ride (DT)
> Call trace:
>  dump_backtrace.part.8+0x98/0xf0
>  show_stack+0x14/0x1c
>  dump_stack_lvl+0x74/0x88
>  dump_stack+0x14/0x1c
>  sysfs_warn_dup+0x60/0x78
>  sysfs_create_dir_ns+0xe8/0x100
>  kobject_add_internal+0x94/0x224
>  kobject_add+0xa8/0x118
>  device_add+0x298/0x7b4
>  platform_device_add+0x1a0/0x228
>  platform_device_register_full+0x11c/0x148
>  dwc_pcie_register_dev+0x74/0xf0 [dwc_pcie_pmu]
>  dwc_pcie_pmu_init+0x7c/0x1000 [dwc_pcie_pmu]
>  do_one_initcall+0x58/0x1c0
>  do_init_module+0x58/0x208
>  load_module+0x1804/0x188c
>  __do_sys_init_module+0x18c/0x1f0
>  __arm64_sys_init_module+0x14/0x1c
>  invoke_syscall+0x40/0xf8
>  el0_svc_common.constprop.1+0x70/0xf4
>  do_el0_svc+0x18/0x20
>  el0_svc+0x28/0xb0
>  el0t_64_sync_handler+0x9c/0xc0
>  el0t_64_sync+0x160/0x164
> kobject: kobject_add_internal failed for dwc_pcie_pmu.0 with -EEXIST,
> don't try to register things with the same name in the same directory.
> 
> This is because of having same bdf value for devices under two different
> controllers.
> 
> Update the logic to use sbdf which is a unique number in case of
> multi instance also.
> 
> Fixes: af9597adc2f1 ("drivers/perf: add DesignWare PCIe PMU driver")

Did you run into this on a QCOM platform with Patch 4/4 since there's
multiple PCIe domains?

> Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com>
> ---
>  drivers/perf/dwc_pcie_pmu.c | 16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/perf/dwc_pcie_pmu.c b/drivers/perf/dwc_pcie_pmu.c
> index c5e328f23841..c115348b8d53 100644
> --- a/drivers/perf/dwc_pcie_pmu.c
> +++ b/drivers/perf/dwc_pcie_pmu.c
> @@ -556,10 +556,10 @@ static int dwc_pcie_register_dev(struct pci_dev *pdev)
>  {
>  	struct platform_device *plat_dev;
>  	struct dwc_pcie_dev_info *dev_info;
> -	u32 bdf;
> +	u32 sbdf;
>  
> -	bdf = PCI_DEVID(pdev->bus->number, pdev->devfn);
> -	plat_dev = platform_device_register_data(NULL, "dwc_pcie_pmu", bdf,
> +	sbdf = (pci_domain_nr(pdev->bus) << 16) | PCI_DEVID(pdev->bus->number, pdev->devfn);
> +	plat_dev = platform_device_register_data(NULL, "dwc_pcie_pmu", sbdf,
>  						 pdev, sizeof(*pdev));
>  
>  	if (IS_ERR(plat_dev))
> @@ -611,15 +611,15 @@ static int dwc_pcie_pmu_probe(struct platform_device *plat_dev)
>  	struct pci_dev *pdev = plat_dev->dev.platform_data;
>  	struct dwc_pcie_pmu *pcie_pmu;
>  	char *name;
> -	u32 bdf, val;
> +	u32 sbdf, val;
>  	u16 vsec;
>  	int ret;
>  
>  	vsec = pci_find_vsec_capability(pdev, pdev->vendor,
>  					DWC_PCIE_VSEC_RAS_DES_ID);
>  	pci_read_config_dword(pdev, vsec + PCI_VNDR_HEADER, &val);
> -	bdf = PCI_DEVID(pdev->bus->number, pdev->devfn);
> -	name = devm_kasprintf(&plat_dev->dev, GFP_KERNEL, "dwc_rootport_%x", bdf);
> +	sbdf = (pci_domain_nr(pdev->bus) << 16) | PCI_DEVID(pdev->bus->number, pdev->devfn);

sbdf is also registerd as the id of the platform device in platform_device_register_data() above,
can we use it directly here without encoding it again?

Thanks.

> +	name = devm_kasprintf(&plat_dev->dev, GFP_KERNEL, "dwc_rootport_%x", sbdf);
>  	if (!name)
>  		return -ENOMEM;
>  
> @@ -650,7 +650,7 @@ static int dwc_pcie_pmu_probe(struct platform_device *plat_dev)
>  	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);
> +		pci_err(pdev, "Error %d registering hotplug @%x\n", ret, sbdf);
>  		return ret;
>  	}
>  
> @@ -663,7 +663,7 @@ static int dwc_pcie_pmu_probe(struct platform_device *plat_dev)
>  
>  	ret = perf_pmu_register(&pcie_pmu->pmu, name, -1);
>  	if (ret) {
> -		pci_err(pdev, "Error %d registering PMU @%x\n", ret, bdf);
> +		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,
> 

Re: [PATCH 2/4] Documentation: dwc_pcie_pmu: Update bdf to sbdf

[Yicong Yang]

On 2024/7/31 12:23, Krishna chaitanya chundru wrote:
> Update document to reflect the driver change to use sbdf instead
> of bdf alone.
> 
> Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com>

Reviewed-by: Yicong Yang <yangyicong@hisilicon.com>

> ---
>  Documentation/admin-guide/perf/dwc_pcie_pmu.rst | 16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/Documentation/admin-guide/perf/dwc_pcie_pmu.rst b/Documentation/admin-guide/perf/dwc_pcie_pmu.rst
> index d47cd229d710..39b8e1fdd0cd 100644
> --- a/Documentation/admin-guide/perf/dwc_pcie_pmu.rst
> +++ b/Documentation/admin-guide/perf/dwc_pcie_pmu.rst
> @@ -46,16 +46,16 @@ Some of the events only exist for specific configurations.
>  DesignWare Cores (DWC) PCIe PMU Driver
>  =======================================
>  
> -This driver adds PMU devices for each PCIe Root Port named based on the BDF of
> +This driver adds PMU devices for each PCIe Root Port named based on the SBDF of
>  the Root Port. For example,
>  
> -    30:03.0 PCI bridge: Device 1ded:8000 (rev 01)
> +    0001:30:03.0 PCI bridge: Device 1ded:8000 (rev 01)
>  
> -the PMU device name for this Root Port is dwc_rootport_3018.
> +the PMU device name for this Root Port is dwc_rootport_13018.
>  
>  The DWC PCIe PMU driver registers a perf PMU driver, which provides
>  description of available events and configuration options in sysfs, see
> -/sys/bus/event_source/devices/dwc_rootport_{bdf}.
> +/sys/bus/event_source/devices/dwc_rootport_{sbdf}.
>  
>  The "format" directory describes format of the config fields of the
>  perf_event_attr structure. The "events" directory provides configuration
> @@ -66,16 +66,16 @@ The "perf list" command shall list the available events from sysfs, e.g.::
>  
>      $# perf list | grep dwc_rootport
>      <...>
> -    dwc_rootport_3018/Rx_PCIe_TLP_Data_Payload/        [Kernel PMU event]
> +    dwc_rootport_13018/Rx_PCIe_TLP_Data_Payload/        [Kernel PMU event]
>      <...>
> -    dwc_rootport_3018/rx_memory_read,lane=?/               [Kernel PMU event]
> +    dwc_rootport_13018/rx_memory_read,lane=?/               [Kernel PMU event]
>  
>  Time Based Analysis Event Usage
>  -------------------------------
>  
>  Example usage of counting PCIe RX TLP data payload (Units of bytes)::
>  
> -    $# perf stat -a -e dwc_rootport_3018/Rx_PCIe_TLP_Data_Payload/
> +    $# perf stat -a -e dwc_rootport_13018/Rx_PCIe_TLP_Data_Payload/
>  
>  The average RX/TX bandwidth can be calculated using the following formula:
>  
> @@ -88,7 +88,7 @@ Lane Event Usage
>  Each lane has the same event set and to avoid generating a list of hundreds
>  of events, the user need to specify the lane ID explicitly, e.g.::
>  
> -    $# perf stat -a -e dwc_rootport_3018/rx_memory_read,lane=4/
> +    $# perf stat -a -e dwc_rootport_13018/rx_memory_read,lane=4/
>  
>  The driver does not support sampling, therefore "perf record" will not
>  work. Per-task (without "-a") perf sessions are not supported.
> 

Re: [PATCH 3/4] perf/dwc_pcie: Always register for PCIe bus notifier

[Yicong Yang]

On 2024/7/31 12:23, Krishna chaitanya chundru wrote:
> When the PCIe devices are discovered late, the driver can't find
> the PCIe devices and returns in the init without registering with
> the bus notifier. Due to that the devices which are discovered late
> the driver can't register for this.
> 
> Register for bus notifier even if the device is not found in init.
> 
> Fixes: af9597adc2f1 ("drivers/perf: add DesignWare PCIe PMU driver")
> Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com>
> ---
>  drivers/perf/dwc_pcie_pmu.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/perf/dwc_pcie_pmu.c b/drivers/perf/dwc_pcie_pmu.c
> index c115348b8d53..aa1010b44bcb 100644
> --- a/drivers/perf/dwc_pcie_pmu.c
> +++ b/drivers/perf/dwc_pcie_pmu.c
> @@ -741,8 +741,6 @@ static int __init dwc_pcie_pmu_init(void)
>  
>  		found = true;
>  	}
> -	if (!found)
> -		return -ENODEV;
>  
>  	ret = cpuhp_setup_state_multi(CPUHP_AP_ONLINE_DYN,
>  				      "perf/dwc_pcie_pmu:online",
> @@ -753,9 +751,11 @@ static int __init dwc_pcie_pmu_init(void)
>  
>  	dwc_pcie_pmu_hp_state = ret;
>  
> -	ret = platform_driver_register(&dwc_pcie_pmu_driver);
> -	if (ret)
> -		goto platform_driver_register_err;
> +	if (!found) {
> +		ret = platform_driver_register(&dwc_pcie_pmu_driver);
> +		if (ret)
> +			goto platform_driver_register_err;
> +	}
> 

This doesn't match the commit.

If any device is found at this stage, we cannot use them since you don't
register a driver.

>  	ret = bus_register_notifier(&pci_bus_type, &dwc_pcie_pmu_nb);
>  	if (ret)
> 

Re: [PATCH 4/4] perf/dwc_pcie: Add support for QCOM vendor devices

[Yicong Yang]

On 2024/7/31 12:23, Krishna chaitanya chundru wrote:
> Update the vendor table with QCOM PCIe vendorid.
> 
> Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com>

Reviewed-by: Yicong Yang <yangyicong@hisilicon.com>

> ---
>  drivers/perf/dwc_pcie_pmu.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/perf/dwc_pcie_pmu.c b/drivers/perf/dwc_pcie_pmu.c
> index aa1010b44bcb..ea73ae5c45c5 100644
> --- a/drivers/perf/dwc_pcie_pmu.c
> +++ b/drivers/perf/dwc_pcie_pmu.c
> @@ -107,6 +107,7 @@ struct dwc_pcie_vendor_id {
>  
>  static const struct dwc_pcie_vendor_id dwc_pcie_vendor_ids[] = {
>  	{.vendor_id = PCI_VENDOR_ID_ALIBABA },
> +	{.vendor_id = PCI_VENDOR_ID_QCOM },
>  	{} /* terminator */
>  };
>  
> 

Re: [PATCH 1/4] perf/dwc_pcie: Fix registration issue in multi PCIe controller instances

[Krishna Chaitanya Chundru]

On 8/15/2024 7:10 PM, Yicong Yang wrote:
> On 2024/7/31 12:23, Krishna chaitanya chundru wrote:
>> When there are multiple of instances of PCIe controllers, registration
>> to perf driver fails with this error.
>> sysfs: cannot create duplicate filename '/devices/platform/dwc_pcie_pmu.0'
>> CPU: 0 PID: 166 Comm: modprobe Not tainted 6.10.0-rc2-next-20240607-dirty
>> Hardware name: Qualcomm SA8775P Ride (DT)
>> Call trace:
>>   dump_backtrace.part.8+0x98/0xf0
>>   show_stack+0x14/0x1c
>>   dump_stack_lvl+0x74/0x88
>>   dump_stack+0x14/0x1c
>>   sysfs_warn_dup+0x60/0x78
>>   sysfs_create_dir_ns+0xe8/0x100
>>   kobject_add_internal+0x94/0x224
>>   kobject_add+0xa8/0x118
>>   device_add+0x298/0x7b4
>>   platform_device_add+0x1a0/0x228
>>   platform_device_register_full+0x11c/0x148
>>   dwc_pcie_register_dev+0x74/0xf0 [dwc_pcie_pmu]
>>   dwc_pcie_pmu_init+0x7c/0x1000 [dwc_pcie_pmu]
>>   do_one_initcall+0x58/0x1c0
>>   do_init_module+0x58/0x208
>>   load_module+0x1804/0x188c
>>   __do_sys_init_module+0x18c/0x1f0
>>   __arm64_sys_init_module+0x14/0x1c
>>   invoke_syscall+0x40/0xf8
>>   el0_svc_common.constprop.1+0x70/0xf4
>>   do_el0_svc+0x18/0x20
>>   el0_svc+0x28/0xb0
>>   el0t_64_sync_handler+0x9c/0xc0
>>   el0t_64_sync+0x160/0x164
>> kobject: kobject_add_internal failed for dwc_pcie_pmu.0 with -EEXIST,
>> don't try to register things with the same name in the same directory.
>>
>> This is because of having same bdf value for devices under two different
>> controllers.
>>
>> Update the logic to use sbdf which is a unique number in case of
>> multi instance also.
>>
>> Fixes: af9597adc2f1 ("drivers/perf: add DesignWare PCIe PMU driver")
> 
> Did you run into this on a QCOM platform with Patch 4/4 since there's
> multiple PCIe domains?
> 
Yes we ran this in QCOM platform where it has multiple PCIe instances.
>> Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com>
>> ---
>>   drivers/perf/dwc_pcie_pmu.c | 16 ++++++++--------
>>   1 file changed, 8 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/perf/dwc_pcie_pmu.c b/drivers/perf/dwc_pcie_pmu.c
>> index c5e328f23841..c115348b8d53 100644
>> --- a/drivers/perf/dwc_pcie_pmu.c
>> +++ b/drivers/perf/dwc_pcie_pmu.c
>> @@ -556,10 +556,10 @@ static int dwc_pcie_register_dev(struct pci_dev *pdev)
>>   {
>>   	struct platform_device *plat_dev;
>>   	struct dwc_pcie_dev_info *dev_info;
>> -	u32 bdf;
>> +	u32 sbdf;
>>   
>> -	bdf = PCI_DEVID(pdev->bus->number, pdev->devfn);
>> -	plat_dev = platform_device_register_data(NULL, "dwc_pcie_pmu", bdf,
>> +	sbdf = (pci_domain_nr(pdev->bus) << 16) | PCI_DEVID(pdev->bus->number, pdev->devfn);
>> +	plat_dev = platform_device_register_data(NULL, "dwc_pcie_pmu", sbdf,
>>   						 pdev, sizeof(*pdev));
>>   
>>   	if (IS_ERR(plat_dev))
>> @@ -611,15 +611,15 @@ static int dwc_pcie_pmu_probe(struct platform_device *plat_dev)
>>   	struct pci_dev *pdev = plat_dev->dev.platform_data;
>>   	struct dwc_pcie_pmu *pcie_pmu;
>>   	char *name;
>> -	u32 bdf, val;
>> +	u32 sbdf, val;
>>   	u16 vsec;
>>   	int ret;
>>   
>>   	vsec = pci_find_vsec_capability(pdev, pdev->vendor,
>>   					DWC_PCIE_VSEC_RAS_DES_ID);
>>   	pci_read_config_dword(pdev, vsec + PCI_VNDR_HEADER, &val);
>> -	bdf = PCI_DEVID(pdev->bus->number, pdev->devfn);
>> -	name = devm_kasprintf(&plat_dev->dev, GFP_KERNEL, "dwc_rootport_%x", bdf);
>> +	sbdf = (pci_domain_nr(pdev->bus) << 16) | PCI_DEVID(pdev->bus->number, pdev->devfn);
> 
> sbdf is also registerd as the id of the platform device in platform_device_register_data() above,
> can we use it directly here without encoding it again?
> 
> Thanks.
> 
ack.

- Krishna chaitanya.
>> +	name = devm_kasprintf(&plat_dev->dev, GFP_KERNEL, "dwc_rootport_%x", sbdf);
>>   	if (!name)
>>   		return -ENOMEM;
>>   
>> @@ -650,7 +650,7 @@ static int dwc_pcie_pmu_probe(struct platform_device *plat_dev)
>>   	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);
>> +		pci_err(pdev, "Error %d registering hotplug @%x\n", ret, sbdf);
>>   		return ret;
>>   	}
>>   
>> @@ -663,7 +663,7 @@ static int dwc_pcie_pmu_probe(struct platform_device *plat_dev)
>>   
>>   	ret = perf_pmu_register(&pcie_pmu->pmu, name, -1);
>>   	if (ret) {
>> -		pci_err(pdev, "Error %d registering PMU @%x\n", ret, bdf);
>> +		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,
>>

Re: [PATCH 3/4] perf/dwc_pcie: Always register for PCIe bus notifier

[Krishna Chaitanya Chundru]

On 8/15/2024 7:19 PM, Yicong Yang wrote:
> On 2024/7/31 12:23, Krishna chaitanya chundru wrote:
>> When the PCIe devices are discovered late, the driver can't find
>> the PCIe devices and returns in the init without registering with
>> the bus notifier. Due to that the devices which are discovered late
>> the driver can't register for this.
>>
>> Register for bus notifier even if the device is not found in init.
>>
>> Fixes: af9597adc2f1 ("drivers/perf: add DesignWare PCIe PMU driver")
>> Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com>
>> ---
>>   drivers/perf/dwc_pcie_pmu.c | 10 +++++-----
>>   1 file changed, 5 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/perf/dwc_pcie_pmu.c b/drivers/perf/dwc_pcie_pmu.c
>> index c115348b8d53..aa1010b44bcb 100644
>> --- a/drivers/perf/dwc_pcie_pmu.c
>> +++ b/drivers/perf/dwc_pcie_pmu.c
>> @@ -741,8 +741,6 @@ static int __init dwc_pcie_pmu_init(void)
>>   
>>   		found = true;
>>   	}
>> -	if (!found)
>> -		return -ENODEV;
>>   
>>   	ret = cpuhp_setup_state_multi(CPUHP_AP_ONLINE_DYN,
>>   				      "perf/dwc_pcie_pmu:online",
>> @@ -753,9 +751,11 @@ static int __init dwc_pcie_pmu_init(void)
>>   
>>   	dwc_pcie_pmu_hp_state = ret;
>>   
>> -	ret = platform_driver_register(&dwc_pcie_pmu_driver);
>> -	if (ret)
>> -		goto platform_driver_register_err;
>> +	if (!found) {
>> +		ret = platform_driver_register(&dwc_pcie_pmu_driver);
>> +		if (ret)
>> +			goto platform_driver_register_err;
>> +	}
>>
> 
> This doesn't match the commit.
> > If any device is found at this stage, we cannot use them since you don't
> register a driver.
> 
If the device is not found then only we are skipping platform driver
register otherwise driver will be registering with the platform driver.

- Krishna Chaitanya.
>>   	ret = bus_register_notifier(&pci_bus_type, &dwc_pcie_pmu_nb);
>>   	if (ret)
>>

Re: [PATCH 3/4] perf/dwc_pcie: Always register for PCIe bus notifier

[Yicong Yang]

On 2024/8/16 11:51, Krishna Chaitanya Chundru wrote:
> 
> 
> On 8/15/2024 7:19 PM, Yicong Yang wrote:
>> On 2024/7/31 12:23, Krishna chaitanya chundru wrote:
>>> When the PCIe devices are discovered late, the driver can't find
>>> the PCIe devices and returns in the init without registering with
>>> the bus notifier. Due to that the devices which are discovered late
>>> the driver can't register for this.
>>>
>>> Register for bus notifier even if the device is not found in init.
>>>
>>> Fixes: af9597adc2f1 ("drivers/perf: add DesignWare PCIe PMU driver")
>>> Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com>
>>> ---
>>>   drivers/perf/dwc_pcie_pmu.c | 10 +++++-----
>>>   1 file changed, 5 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/drivers/perf/dwc_pcie_pmu.c b/drivers/perf/dwc_pcie_pmu.c
>>> index c115348b8d53..aa1010b44bcb 100644
>>> --- a/drivers/perf/dwc_pcie_pmu.c
>>> +++ b/drivers/perf/dwc_pcie_pmu.c
>>> @@ -741,8 +741,6 @@ static int __init dwc_pcie_pmu_init(void)
>>>             found = true;
>>>       }
>>> -    if (!found)
>>> -        return -ENODEV;
>>>         ret = cpuhp_setup_state_multi(CPUHP_AP_ONLINE_DYN,
>>>                         "perf/dwc_pcie_pmu:online",
>>> @@ -753,9 +751,11 @@ static int __init dwc_pcie_pmu_init(void)
>>>         dwc_pcie_pmu_hp_state = ret;
>>>   -    ret = platform_driver_register(&dwc_pcie_pmu_driver);
>>> -    if (ret)
>>> -        goto platform_driver_register_err;
>>> +    if (!found) {
>>> +        ret = platform_driver_register(&dwc_pcie_pmu_driver);
>>> +        if (ret)
>>> +            goto platform_driver_register_err;
>>> +    }
>>>
>>
>> This doesn't match the commit.
>> > If any device is found at this stage, we cannot use them since you don't
>> register a driver.
>>
> If the device is not found then only we are skipping platform driver
> register otherwise driver will be registering with the platform driver.
> 

think about the case that devices already discovered before module init.
without the change here we'll register both the platform devices and driver
but with the change here we'll only register the platform devices without
the related driver to probe them.

Try to register the driver and notifier unconditionally will solve the issue.
It'll probe the device and register the PMU if later device is added by
the bus notifier.

Thanks.

> - Krishna Chaitanya.
>>>       ret = bus_register_notifier(&pci_bus_type, &dwc_pcie_pmu_nb);
>>>       if (ret)
>>>
> 
> .