public inbox for linux-pci@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] PCI: dwc-debugfs: Couple of fixes
@ 2025-02-25 17:12 Manivannan Sadhasivam
  2025-02-25 17:12 ` [PATCH 1/2] PCI: dwc-debugfs: Perform deinit only when the debugfs is initialized Manivannan Sadhasivam
                   ` (3 more replies)
  0 siblings, 4 replies; 9+ messages in thread
From: Manivannan Sadhasivam @ 2025-02-25 17:12 UTC (permalink / raw)
  To: jingoohan1, lpieralisi, kw
  Cc: robh, bhelgaas, linux-pci, linux-kernel, shradha.t, cassel,
	Manivannan Sadhasivam

Hi,

This series has a couple of fixes for the recently merged debugfs patches.
This series is rebased on top of pci/controller/dwc branch.

- Mani

Manivannan Sadhasivam (2):
  PCI: dwc-debugfs: Perform deinit only when the debugfs is initialized
  PCI: dwc-debugfs: Return -EOPNOTSUPP if an event counter is not
    supported

 .../controller/dwc/pcie-designware-debugfs.c   | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

-- 
2.25.1


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 1/2] PCI: dwc-debugfs: Perform deinit only when the debugfs is initialized
  2025-02-25 17:12 [PATCH 0/2] PCI: dwc-debugfs: Couple of fixes Manivannan Sadhasivam
@ 2025-02-25 17:12 ` Manivannan Sadhasivam
  2025-02-26  6:53   ` Krzysztof Wilczyński
  2025-02-25 17:12 ` [PATCH 2/2] PCI: dwc-debugfs: Return -EOPNOTSUPP if an event counter is not supported Manivannan Sadhasivam
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 9+ messages in thread
From: Manivannan Sadhasivam @ 2025-02-25 17:12 UTC (permalink / raw)
  To: jingoohan1, lpieralisi, kw
  Cc: robh, bhelgaas, linux-pci, linux-kernel, shradha.t, cassel,
	Manivannan Sadhasivam

Some endpoint controller drivers like pcie-qcom-ep, pcie-tegra194 call
dw_pcie_ep_cleanup() to cleanup the resources at the start of the PERST#
deassert (due to refclk dependency). By that time, debugfs won't be
registered, leading to NULL pointer dereference:

Unable to handle kernel NULL pointer dereference at virtual address 0000000000000000
Call trace:
dwc_pcie_debugfs_deinit+0x18/0x38 (P)
dw_pcie_ep_cleanup+0x2c/0x50
qcom_pcie_ep_perst_irq_thread+0x278/0x5e8

So perform deinit only when the debugfs is initialized.

Fixes: 24c117c60658 ("PCI: dwc: Add debugfs based Silicon Debug support for DWC")
Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
---
 drivers/pci/controller/dwc/pcie-designware-debugfs.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/pci/controller/dwc/pcie-designware-debugfs.c b/drivers/pci/controller/dwc/pcie-designware-debugfs.c
index dca1e9999113..9ff4d45e80f1 100644
--- a/drivers/pci/controller/dwc/pcie-designware-debugfs.c
+++ b/drivers/pci/controller/dwc/pcie-designware-debugfs.c
@@ -535,6 +535,9 @@ static int dwc_pcie_rasdes_debugfs_init(struct dw_pcie *pci, struct dentry *dir)
 
 void dwc_pcie_debugfs_deinit(struct dw_pcie *pci)
 {
+	if (!pci->debugfs)
+		return;
+
 	dwc_pcie_rasdes_debugfs_deinit(pci);
 	debugfs_remove_recursive(pci->debugfs->debug_dir);
 }
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 2/2] PCI: dwc-debugfs: Return -EOPNOTSUPP if an event counter is not supported
  2025-02-25 17:12 [PATCH 0/2] PCI: dwc-debugfs: Couple of fixes Manivannan Sadhasivam
  2025-02-25 17:12 ` [PATCH 1/2] PCI: dwc-debugfs: Perform deinit only when the debugfs is initialized Manivannan Sadhasivam
@ 2025-02-25 17:12 ` Manivannan Sadhasivam
  2025-02-26  6:56   ` Krzysztof Wilczyński
  2025-02-26  1:55 ` [PATCH 0/2] PCI: dwc-debugfs: Couple of fixes Hans Zhang
  2025-02-26  7:17 ` Krzysztof Wilczyński
  3 siblings, 1 reply; 9+ messages in thread
From: Manivannan Sadhasivam @ 2025-02-25 17:12 UTC (permalink / raw)
  To: jingoohan1, lpieralisi, kw
  Cc: robh, bhelgaas, linux-pci, linux-kernel, shradha.t, cassel,
	Manivannan Sadhasivam

If the platform doesn't support an event counter, enabling it using the
'counter_enable' debugfs attribute currently will succeed. But reading the
debugfs attribute back will return 'Counter Disabled'.

This could cause confusion to the users. So while enabling an event
counter in counter_enable_write(), always read back the status to check if
the counter is enabled or not. If not, return -EOPNOTSUPP to let the users
know that the event counter is not supported.

Fixes: 9f99c304c467 ("PCI: dwc: Add debugfs based Statistical Counter support for DWC")
Suggested-by: Niklas Cassel <cassel@kernel.org>
Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
---
 .../pci/controller/dwc/pcie-designware-debugfs.c  | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/drivers/pci/controller/dwc/pcie-designware-debugfs.c b/drivers/pci/controller/dwc/pcie-designware-debugfs.c
index 9ff4d45e80f1..1f1bd9888327 100644
--- a/drivers/pci/controller/dwc/pcie-designware-debugfs.c
+++ b/drivers/pci/controller/dwc/pcie-designware-debugfs.c
@@ -352,6 +352,21 @@ static ssize_t counter_enable_write(struct file *file, const char __user *buf,
 		val |= FIELD_PREP(EVENT_COUNTER_ENABLE, PER_EVENT_OFF);
 
 	dw_pcie_writel_dbi(pci, rinfo->ras_cap_offset + RAS_DES_EVENT_COUNTER_CTRL_REG, val);
+
+	/*
+	 * While enabling the counter, always read back the status to check if
+	 * it is enabled or not. Return error if it is not enabled to let the
+	 * users know that the counter is not supported on the platform.
+	 */
+	if (enable) {
+		val = dw_pcie_readl_dbi(pci, rinfo->ras_cap_offset +
+					RAS_DES_EVENT_COUNTER_CTRL_REG);
+		if (!FIELD_GET(EVENT_COUNTER_STATUS, val)) {
+			mutex_unlock(&rinfo->reg_event_lock);
+			return -EOPNOTSUPP;
+		}
+	}
+
 	mutex_unlock(&rinfo->reg_event_lock);
 
 	return count;
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH 0/2] PCI: dwc-debugfs: Couple of fixes
  2025-02-25 17:12 [PATCH 0/2] PCI: dwc-debugfs: Couple of fixes Manivannan Sadhasivam
  2025-02-25 17:12 ` [PATCH 1/2] PCI: dwc-debugfs: Perform deinit only when the debugfs is initialized Manivannan Sadhasivam
  2025-02-25 17:12 ` [PATCH 2/2] PCI: dwc-debugfs: Return -EOPNOTSUPP if an event counter is not supported Manivannan Sadhasivam
@ 2025-02-26  1:55 ` Hans Zhang
  2025-02-26  7:43   ` Krzysztof Wilczyński
  2025-02-26  7:17 ` Krzysztof Wilczyński
  3 siblings, 1 reply; 9+ messages in thread
From: Hans Zhang @ 2025-02-26  1:55 UTC (permalink / raw)
  To: Manivannan Sadhasivam, jingoohan1, lpieralisi, kw
  Cc: robh, bhelgaas, linux-pci, linux-kernel, shradha.t, cassel

Hi Mani,

Can you submit after this patch? Otherwise, will my patch conflict?

https://patchwork.kernel.org/project/linux-pci/patch/20250223141848.231232-1-18255117159@163.com/

Best regards
Hans

On 2025/2/26 01:12, Manivannan Sadhasivam wrote:
> Hi,
> 
> This series has a couple of fixes for the recently merged debugfs patches.
> This series is rebased on top of pci/controller/dwc branch.
> 
> - Mani
> 
> Manivannan Sadhasivam (2):
>    PCI: dwc-debugfs: Perform deinit only when the debugfs is initialized
>    PCI: dwc-debugfs: Return -EOPNOTSUPP if an event counter is not
>      supported
> 
>   .../controller/dwc/pcie-designware-debugfs.c   | 18 ++++++++++++++++++
>   1 file changed, 18 insertions(+)


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/2] PCI: dwc-debugfs: Perform deinit only when the debugfs is initialized
  2025-02-25 17:12 ` [PATCH 1/2] PCI: dwc-debugfs: Perform deinit only when the debugfs is initialized Manivannan Sadhasivam
@ 2025-02-26  6:53   ` Krzysztof Wilczyński
  0 siblings, 0 replies; 9+ messages in thread
From: Krzysztof Wilczyński @ 2025-02-26  6:53 UTC (permalink / raw)
  To: Manivannan Sadhasivam
  Cc: jingoohan1, lpieralisi, robh, bhelgaas, linux-pci, linux-kernel,
	shradha.t, cassel

Hello,

> Some endpoint controller drivers like pcie-qcom-ep, pcie-tegra194 call
> dw_pcie_ep_cleanup() to cleanup the resources at the start of the PERST#
> deassert (due to refclk dependency). By that time, debugfs won't be
> registered, leading to NULL pointer dereference:
> 
> Unable to handle kernel NULL pointer dereference at virtual address 0000000000000000
> Call trace:
> dwc_pcie_debugfs_deinit+0x18/0x38 (P)
> dw_pcie_ep_cleanup+0x2c/0x50
> qcom_pcie_ep_perst_irq_thread+0x278/0x5e8
> 
> So perform deinit only when the debugfs is initialized.

Good catch!  With that...

Reviewed-by: Krzysztof Wilczyński <kwilczynski@kernel.org>

Thank you!

	Krzysztof

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/2] PCI: dwc-debugfs: Return -EOPNOTSUPP if an event counter is not supported
  2025-02-25 17:12 ` [PATCH 2/2] PCI: dwc-debugfs: Return -EOPNOTSUPP if an event counter is not supported Manivannan Sadhasivam
@ 2025-02-26  6:56   ` Krzysztof Wilczyński
  0 siblings, 0 replies; 9+ messages in thread
From: Krzysztof Wilczyński @ 2025-02-26  6:56 UTC (permalink / raw)
  To: Manivannan Sadhasivam
  Cc: jingoohan1, lpieralisi, robh, bhelgaas, linux-pci, linux-kernel,
	shradha.t, cassel

Hello,

> If the platform doesn't support an event counter, enabling it using the
> 'counter_enable' debugfs attribute currently will succeed. But reading the
> debugfs attribute back will return 'Counter Disabled'.
> 
> This could cause confusion to the users. So while enabling an event
> counter in counter_enable_write(), always read back the status to check if
> the counter is enabled or not. If not, return -EOPNOTSUPP to let the users
> know that the event counter is not supported.

Thank you for following up on this.  Appreciated.  With that...

Reviewed-by: Krzysztof Wilczyński <kwilczynski@kernel.org>

Thank you!

	Krzysztof

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 0/2] PCI: dwc-debugfs: Couple of fixes
  2025-02-25 17:12 [PATCH 0/2] PCI: dwc-debugfs: Couple of fixes Manivannan Sadhasivam
                   ` (2 preceding siblings ...)
  2025-02-26  1:55 ` [PATCH 0/2] PCI: dwc-debugfs: Couple of fixes Hans Zhang
@ 2025-02-26  7:17 ` Krzysztof Wilczyński
  3 siblings, 0 replies; 9+ messages in thread
From: Krzysztof Wilczyński @ 2025-02-26  7:17 UTC (permalink / raw)
  To: Manivannan Sadhasivam
  Cc: jingoohan1, lpieralisi, robh, bhelgaas, linux-pci, linux-kernel,
	shradha.t, cassel

Hello,

> This series has a couple of fixes for the recently merged debugfs patches.
> This series is rebased on top of pci/controller/dwc branch.

Squashed both patches with the changes already on the branch.

Thank you for both fixes!

	Krzysztof

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 0/2] PCI: dwc-debugfs: Couple of fixes
  2025-02-26  1:55 ` [PATCH 0/2] PCI: dwc-debugfs: Couple of fixes Hans Zhang
@ 2025-02-26  7:43   ` Krzysztof Wilczyński
  2025-02-26  7:45     ` Hans Zhang
  0 siblings, 1 reply; 9+ messages in thread
From: Krzysztof Wilczyński @ 2025-02-26  7:43 UTC (permalink / raw)
  To: Hans Zhang
  Cc: Manivannan Sadhasivam, jingoohan1, lpieralisi, robh, bhelgaas,
	linux-pci, linux-kernel, shradha.t, cassel

Hello,

> Can you submit after this patch? Otherwise, will my patch conflict?
> 
> https://patchwork.kernel.org/project/linux-pci/patch/20250223141848.231232-1-18255117159@163.com/

No worries.  We can resolve conflicts while applying. :)

	Krzysztof

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 0/2] PCI: dwc-debugfs: Couple of fixes
  2025-02-26  7:43   ` Krzysztof Wilczyński
@ 2025-02-26  7:45     ` Hans Zhang
  0 siblings, 0 replies; 9+ messages in thread
From: Hans Zhang @ 2025-02-26  7:45 UTC (permalink / raw)
  To: Krzysztof Wilczyński
  Cc: Manivannan Sadhasivam, jingoohan1, lpieralisi, robh, bhelgaas,
	linux-pci, linux-kernel, shradha.t, cassel



On 2025/2/26 15:43, Krzysztof Wilczyński wrote:
> Hello,
> 
>> Can you submit after this patch? Otherwise, will my patch conflict?
>>
>> https://patchwork.kernel.org/project/linux-pci/patch/20250223141848.231232-1-18255117159@163.com/
> 
> No worries.  We can resolve conflicts while applying. :)
> 

I see, thanks Krzysztof for replying.

Best regards
Hans


^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2025-02-26  7:46 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-02-25 17:12 [PATCH 0/2] PCI: dwc-debugfs: Couple of fixes Manivannan Sadhasivam
2025-02-25 17:12 ` [PATCH 1/2] PCI: dwc-debugfs: Perform deinit only when the debugfs is initialized Manivannan Sadhasivam
2025-02-26  6:53   ` Krzysztof Wilczyński
2025-02-25 17:12 ` [PATCH 2/2] PCI: dwc-debugfs: Return -EOPNOTSUPP if an event counter is not supported Manivannan Sadhasivam
2025-02-26  6:56   ` Krzysztof Wilczyński
2025-02-26  1:55 ` [PATCH 0/2] PCI: dwc-debugfs: Couple of fixes Hans Zhang
2025-02-26  7:43   ` Krzysztof Wilczyński
2025-02-26  7:45     ` Hans Zhang
2025-02-26  7:17 ` Krzysztof Wilczyński

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox