[PATCH 0/2] PCI: Keystone: __init and IRQ Fixes

public inbox for linux-arm-kernel@lists.infradead.org
 help / color / mirror / Atom feed

* [PATCH 0/2] PCI: Keystone: __init and IRQ Fixes
@ 2025-09-12 10:07 Siddharth Vadapalli
  2025-09-12 10:07 ` [PATCH 1/2] PCI: keystone: Use devm_request_irq() to free "ks-pcie-error-irq" on exit Siddharth Vadapalli
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Siddharth Vadapalli @ 2025-09-12 10:07 UTC (permalink / raw)
  To: lpieralisi, kwilczynski, mani, robh, bhelgaas, cassel, kishon,
	sergio.paracuellos, 18255117159, jirislaby, m-karicheri2,
	santosh.shilimkar
  Cc: stable, linux-pci, linux-kernel, linux-arm-kernel, srk,
	s-vadapalli

Hello,

This series is based on commit
320475fbd590 Merge tag 'mtd/fixes-for-6.17-rc6' of git://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux
of Mainline Linux.

The first patch in the series has been posted as a Fix in contrast to
its predecessor at:
https://lore.kernel.org/r/20250903124505.365913-10-s-vadapalli@ti.com/
based on the feedback provided by Jiri Slaby <jirislaby@kernel.org> at:
https://lore.kernel.org/r/3d3a4b52-e343-42f3-9d69-94c259812143@kernel.org/
Since the Fix is independent of enabling loadable module support for the
pci-keystone.c driver, it is being posted as a new patch.

Checking out at the commit of Mainline Linux which this series is based
on, I noticed an exception triggered by the pci-keystone.c driver during
its probe. Although this is not a fatal exception and Linux continues to
boot, the driver is non-functional. I root-caused the exception to
free_initmem() freeing the memory associated with the ks_pcie_host_init()
function in the driver before the driver's probe was invoked. This
appears to be a race condition but it is easily reproducible with the
Linux .config that I have used. The fix therefore is to remove the
__init macro which is implemented by the second patch in the series.

For reference, the logs for the case where Linux is built by checking
out at the base commit of Mainline Linux are:
https://gist.github.com/Siddharth-Vadapalli-at-TI/f4891b707921c53dfb464ad2f3a968bf
and the logs clearly prove that the print associated with free_initmem()
which is:
[    2.446834] Freeing unused kernel memory: 4864K
is displayed prior to the prints associated with the pci-keystone.c
driver being probed which is:
[    7.707103] keystone-pcie 5500000.pcie: host bridge /bus@100000/pcie@5500000 ranges:

Building Linux by applying both patches in the series on the base commit of
Mainline Linux, the driver probes successfully without any exceptions or
errors. This was tested on AM654-EVM with an NVMe SSD connected to the
PCIe Connector on the board. The NVMe SSD enumerates successfully.
Additionally, the 'hdparm' utility was used to read from the SSD
confirming that the SSD is functional. The logs corresponding to this are:
https://gist.github.com/Siddharth-Vadapalli-at-TI/1b09a12a53db4233e82c5bcfc0e89214

Regards,
Siddharth.

Siddharth Vadapalli (2):
  PCI: keystone: Use devm_request_irq() to free "ks-pcie-error-irq" on
    exit
  PCI: keystone: Remove the __init macro for the ks_pcie_host_init()
    callback

 drivers/pci/controller/dwc/pci-keystone.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

-- 
2.43.0

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

* [PATCH 1/2] PCI: keystone: Use devm_request_irq() to free "ks-pcie-error-irq" on exit
  2025-09-12 10:07 [PATCH 0/2] PCI: Keystone: __init and IRQ Fixes Siddharth Vadapalli
@ 2025-09-12 10:07 ` Siddharth Vadapalli
  2025-09-12 10:07 ` [PATCH 2/2] PCI: keystone: Remove the __init macro for the ks_pcie_host_init() callback Siddharth Vadapalli
  2025-09-29 16:25 ` [PATCH 0/2] PCI: Keystone: __init and IRQ Fixes Manivannan Sadhasivam
  2 siblings, 0 replies; 6+ messages in thread
From: Siddharth Vadapalli @ 2025-09-12 10:07 UTC (permalink / raw)
  To: lpieralisi, kwilczynski, mani, robh, bhelgaas, cassel, kishon,
	sergio.paracuellos, 18255117159, jirislaby, m-karicheri2,
	santosh.shilimkar
  Cc: stable, linux-pci, linux-kernel, linux-arm-kernel, srk,
	s-vadapalli

Commit under Fixes introduced the IRQ handler for "ks-pcie-error-irq".
The interrupt is acquired using "request_irq()" but is never freed if
the driver exits due to an error. Although the section in the driver that
invokes "request_irq()" has moved around over time, the issue hasn't been
addressed until now.

Fix this by using "devm_request_irq()" which shall automatically release
the interrupt if the driver exits. Also, since the interrupt handler for
the "ks-pcie-error-irq" namely "ks_pcie_handle_error_irq() is only printing
the error and clearing the interrupt, there is no necessity to prefer
devm_request_threaded_irq() over devm_request_irq().

Fixes: 025dd3daeda7 ("PCI: keystone: Add error IRQ handler")
Reported-by: Jiri Slaby <jirislaby@kernel.org>
Closes: https://lore.kernel.org/r/3d3a4b52-e343-42f3-9d69-94c259812143@kernel.org
Cc: <stable@vger.kernel.org>
Signed-off-by: Siddharth Vadapalli <s-vadapalli@ti.com>
---
 drivers/pci/controller/dwc/pci-keystone.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/controller/dwc/pci-keystone.c b/drivers/pci/controller/dwc/pci-keystone.c
index 2b2632e513b5..21808a9e5158 100644
--- a/drivers/pci/controller/dwc/pci-keystone.c
+++ b/drivers/pci/controller/dwc/pci-keystone.c
@@ -1201,8 +1201,8 @@ static int ks_pcie_probe(struct platform_device *pdev)
 	if (irq < 0)
 		return irq;
 
-	ret = request_irq(irq, ks_pcie_err_irq_handler, IRQF_SHARED,
-			  "ks-pcie-error-irq", ks_pcie);
+	ret = devm_request_irq(dev, irq, ks_pcie_err_irq_handler, IRQF_SHARED,
+			       "ks-pcie-error-irq", ks_pcie);
 	if (ret < 0) {
 		dev_err(dev, "failed to request error IRQ %d\n",
 			irq);
-- 
2.43.0



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

* [PATCH 2/2] PCI: keystone: Remove the __init macro for the ks_pcie_host_init() callback
  2025-09-12 10:07 [PATCH 0/2] PCI: Keystone: __init and IRQ Fixes Siddharth Vadapalli
  2025-09-12 10:07 ` [PATCH 1/2] PCI: keystone: Use devm_request_irq() to free "ks-pcie-error-irq" on exit Siddharth Vadapalli
@ 2025-09-12 10:07 ` Siddharth Vadapalli
  2025-10-02 14:36   ` Bjorn Helgaas
  2025-09-29 16:25 ` [PATCH 0/2] PCI: Keystone: __init and IRQ Fixes Manivannan Sadhasivam
  2 siblings, 1 reply; 6+ messages in thread
From: Siddharth Vadapalli @ 2025-09-12 10:07 UTC (permalink / raw)
  To: lpieralisi, kwilczynski, mani, robh, bhelgaas, cassel, kishon,
	sergio.paracuellos, 18255117159, jirislaby, m-karicheri2,
	santosh.shilimkar
  Cc: stable, linux-pci, linux-kernel, linux-arm-kernel, srk,
	s-vadapalli

The ks_pcie_host_init() callback registered by the driver is invoked by
dw_pcie_host_init(). Since the driver probe is not guaranteed to finish
before the kernel initialization phase, the memory associated with
ks_pcie_host_init() may already be freed by free_initmem().

It is observed in practice that the print associated with free_initmem()
which is:
	"Freeing unused kernel memory: ..."
is displayed before the driver is probed, following which an exception is
triggered when ks_pcie_host_init() is invoked which looks like:

	Unable to handle kernel paging request at virtual address ...
	Mem abort info:
	...
	pc : ks_pcie_host_init+0x0/0x540
	lr : dw_pcie_host_init+0x170/0x498
	...
	ks_pcie_host_init+0x0/0x540 (P)
	ks_pcie_probe+0x728/0x84c
	platform_probe+0x5c/0x98
	really_probe+0xbc/0x29c
	__driver_probe_device+0x78/0x12c
	driver_probe_device+0xd8/0x15c
	...

Fix this by removing the "__init" macro associated with the
ks_pcie_host_init() callback and the ks_pcie_init_id() function that it
internally invokes.

Fixes: 0c4ffcfe1fbc ("PCI: keystone: Add TI Keystone PCIe driver")
Cc: <stable@vger.kernel.org>
Signed-off-by: Siddharth Vadapalli <s-vadapalli@ti.com>
---
 drivers/pci/controller/dwc/pci-keystone.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/controller/dwc/pci-keystone.c b/drivers/pci/controller/dwc/pci-keystone.c
index 21808a9e5158..c6e082dcb3bc 100644
--- a/drivers/pci/controller/dwc/pci-keystone.c
+++ b/drivers/pci/controller/dwc/pci-keystone.c
@@ -799,7 +799,7 @@ static int ks_pcie_fault(unsigned long addr, unsigned int fsr,
 }
 #endif
 
-static int __init ks_pcie_init_id(struct keystone_pcie *ks_pcie)
+static int ks_pcie_init_id(struct keystone_pcie *ks_pcie)
 {
 	int ret;
 	unsigned int id;
@@ -831,7 +831,7 @@ static int __init ks_pcie_init_id(struct keystone_pcie *ks_pcie)
 	return 0;
 }
 
-static int __init ks_pcie_host_init(struct dw_pcie_rp *pp)
+static int ks_pcie_host_init(struct dw_pcie_rp *pp)
 {
 	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
 	struct keystone_pcie *ks_pcie = to_keystone_pcie(pci);
-- 
2.43.0



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

* Re: [PATCH 2/2] PCI: keystone: Remove the __init macro for the ks_pcie_host_init() callback
  2025-09-12 10:07 ` [PATCH 2/2] PCI: keystone: Remove the __init macro for the ks_pcie_host_init() callback Siddharth Vadapalli
@ 2025-10-02 14:36   ` Bjorn Helgaas
  2025-10-22 10:03     ` Siddharth Vadapalli
  0 siblings, 1 reply; 6+ messages in thread
From: Bjorn Helgaas @ 2025-10-02 14:36 UTC (permalink / raw)
  To: Siddharth Vadapalli
  Cc: lpieralisi, kwilczynski, mani, robh, bhelgaas, cassel, kishon,
	sergio.paracuellos, 18255117159, jirislaby, m-karicheri2,
	santosh.shilimkar, stable, linux-pci, linux-kernel,
	linux-arm-kernel, srk

On Fri, Sep 12, 2025 at 03:37:59PM +0530, Siddharth Vadapalli wrote:
> The ks_pcie_host_init() callback registered by the driver is invoked by
> dw_pcie_host_init(). Since the driver probe is not guaranteed to finish
> before the kernel initialization phase, the memory associated with
> ks_pcie_host_init() may already be freed by free_initmem().
> 
> It is observed in practice that the print associated with free_initmem()
> which is:
> 	"Freeing unused kernel memory: ..."
> is displayed before the driver is probed, following which an exception is
> triggered when ks_pcie_host_init() is invoked which looks like:
> 
> 	Unable to handle kernel paging request at virtual address ...
> 	Mem abort info:
> 	...
> 	pc : ks_pcie_host_init+0x0/0x540
> 	lr : dw_pcie_host_init+0x170/0x498
> 	...
> 	ks_pcie_host_init+0x0/0x540 (P)
> 	ks_pcie_probe+0x728/0x84c
> 	platform_probe+0x5c/0x98
> 	really_probe+0xbc/0x29c
> 	__driver_probe_device+0x78/0x12c
> 	driver_probe_device+0xd8/0x15c
> 	...
> 
> Fix this by removing the "__init" macro associated with the
> ks_pcie_host_init() callback and the ks_pcie_init_id() function that it
> internally invokes.
> 
> Fixes: 0c4ffcfe1fbc ("PCI: keystone: Add TI Keystone PCIe driver")
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Siddharth Vadapalli <s-vadapalli@ti.com>

I dropped this from pci/controller/keystone because of the resulting
section mismatch:

  https://lore.kernel.org/r/202510010726.GPljD7FR-lkp@intel.com

ks_pcie_host_init() calls hook_fault_code(), which is __init, so we
can't make ks_pcie_host_init() non-__init.

Both are bad problems, but there's no point in just swapping one
problem for a different one.

> ---
>  drivers/pci/controller/dwc/pci-keystone.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pci-keystone.c b/drivers/pci/controller/dwc/pci-keystone.c
> index 21808a9e5158..c6e082dcb3bc 100644
> --- a/drivers/pci/controller/dwc/pci-keystone.c
> +++ b/drivers/pci/controller/dwc/pci-keystone.c
> @@ -799,7 +799,7 @@ static int ks_pcie_fault(unsigned long addr, unsigned int fsr,
>  }
>  #endif
>  
> -static int __init ks_pcie_init_id(struct keystone_pcie *ks_pcie)
> +static int ks_pcie_init_id(struct keystone_pcie *ks_pcie)
>  {
>  	int ret;
>  	unsigned int id;
> @@ -831,7 +831,7 @@ static int __init ks_pcie_init_id(struct keystone_pcie *ks_pcie)
>  	return 0;
>  }
>  
> -static int __init ks_pcie_host_init(struct dw_pcie_rp *pp)
> +static int ks_pcie_host_init(struct dw_pcie_rp *pp)
>  {
>  	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
>  	struct keystone_pcie *ks_pcie = to_keystone_pcie(pci);
> -- 
> 2.43.0
> 


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

* Re: [PATCH 2/2] PCI: keystone: Remove the __init macro for the ks_pcie_host_init() callback
  2025-10-02 14:36   ` Bjorn Helgaas
@ 2025-10-22 10:03     ` Siddharth Vadapalli
  0 siblings, 0 replies; 6+ messages in thread
From: Siddharth Vadapalli @ 2025-10-22 10:03 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: lpieralisi, kwilczynski, mani, robh, bhelgaas, cassel, kishon,
	sergio.paracuellos, 18255117159, jirislaby, m-karicheri2,
	santosh.shilimkar, stable, linux-pci, linux-kernel,
	linux-arm-kernel, srk, s-vadapalli

On Thu, 2025-10-02 at 09:36 -0500, Bjorn Helgaas wrote:

Hello Bjorn,

> On Fri, Sep 12, 2025 at 03:37:59PM +0530, Siddharth Vadapalli wrote:
> > The ks_pcie_host_init() callback registered by the driver is invoked by
> > dw_pcie_host_init(). Since the driver probe is not guaranteed to finish
> > before the kernel initialization phase, the memory associated with
> > ks_pcie_host_init() may already be freed by free_initmem().
> > 
> > It is observed in practice that the print associated with free_initmem()
> > which is:
> > 	"Freeing unused kernel memory: ..."
> > is displayed before the driver is probed, following which an exception is
> > triggered when ks_pcie_host_init() is invoked which looks like:
> > 
> > 	Unable to handle kernel paging request at virtual address ...
> > 	Mem abort info:
> > 	...
> > 	pc : ks_pcie_host_init+0x0/0x540
> > 	lr : dw_pcie_host_init+0x170/0x498
> > 	...
> > 	ks_pcie_host_init+0x0/0x540 (P)
> > 	ks_pcie_probe+0x728/0x84c
> > 	platform_probe+0x5c/0x98
> > 	really_probe+0xbc/0x29c
> > 	__driver_probe_device+0x78/0x12c
> > 	driver_probe_device+0xd8/0x15c
> > 	...
> > 
> > Fix this by removing the "__init" macro associated with the
> > ks_pcie_host_init() callback and the ks_pcie_init_id() function that it
> > internally invokes.
> > 
> > Fixes: 0c4ffcfe1fbc ("PCI: keystone: Add TI Keystone PCIe driver")
> > Cc: <stable@vger.kernel.org>
> > Signed-off-by: Siddharth Vadapalli <s-vadapalli@ti.com>
> 
> I dropped this from pci/controller/keystone because of the resulting
> section mismatch:
> 
>   https://lore.kernel.org/r/202510010726.GPljD7FR-lkp@intel.com
> 
> ks_pcie_host_init() calls hook_fault_code(), which is __init, so we
> can't make ks_pcie_host_init() non-__init.
> 
> Both are bad problems, but there's no point in just swapping one
> problem for a different one.

Since this patch is required only for the case where the driver supports
being built as a loadable module, I have reworked on the patch and have
squashed it into patch 4 of the following series:
https://lore.kernel.org/r/20251022095724.997218-5-s-vadapalli@ti.com/
The implementation above ensures that 'hook_fault_code()' is placed within
an '__init' function while the '__init' keywords can safely be removed from
the remaining functions. Please review and let me know.

Regards,
Siddharth.


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

* Re: [PATCH 0/2] PCI: Keystone: __init and IRQ Fixes
  2025-09-12 10:07 [PATCH 0/2] PCI: Keystone: __init and IRQ Fixes Siddharth Vadapalli
  2025-09-12 10:07 ` [PATCH 1/2] PCI: keystone: Use devm_request_irq() to free "ks-pcie-error-irq" on exit Siddharth Vadapalli
  2025-09-12 10:07 ` [PATCH 2/2] PCI: keystone: Remove the __init macro for the ks_pcie_host_init() callback Siddharth Vadapalli
@ 2025-09-29 16:25 ` Manivannan Sadhasivam
  2 siblings, 0 replies; 6+ messages in thread
From: Manivannan Sadhasivam @ 2025-09-29 16:25 UTC (permalink / raw)
  To: lpieralisi, kwilczynski, robh, bhelgaas, cassel, kishon,
	sergio.paracuellos, 18255117159, jirislaby, m-karicheri2,
	santosh.shilimkar, Siddharth Vadapalli
  Cc: stable, linux-pci, linux-kernel, linux-arm-kernel, srk


On Fri, 12 Sep 2025 15:37:57 +0530, Siddharth Vadapalli wrote:
> This series is based on commit
> 320475fbd590 Merge tag 'mtd/fixes-for-6.17-rc6' of git://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux
> of Mainline Linux.
> 
> The first patch in the series has been posted as a Fix in contrast to
> its predecessor at:
> https://lore.kernel.org/r/20250903124505.365913-10-s-vadapalli@ti.com/
> based on the feedback provided by Jiri Slaby <jirislaby@kernel.org> at:
> https://lore.kernel.org/r/3d3a4b52-e343-42f3-9d69-94c259812143@kernel.org/
> Since the Fix is independent of enabling loadable module support for the
> pci-keystone.c driver, it is being posted as a new patch.
> 
> [...]

Applied, thanks!

[1/2] PCI: keystone: Use devm_request_irq() to free "ks-pcie-error-irq" on exit
      commit: e51d05f523e43ce5d2bad957943a2b14f68078cd
[2/2] PCI: keystone: Remove the __init macro for the ks_pcie_host_init() callback
      commit: 860daf4ba3c034995bafa4c3756942262a9cd32d

Best regards,
-- 
Manivannan Sadhasivam <mani@kernel.org>



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

end of thread, other threads:[~2025-10-22 10:04 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-09-12 10:07 [PATCH 0/2] PCI: Keystone: __init and IRQ Fixes Siddharth Vadapalli
2025-09-12 10:07 ` [PATCH 1/2] PCI: keystone: Use devm_request_irq() to free "ks-pcie-error-irq" on exit Siddharth Vadapalli
2025-09-12 10:07 ` [PATCH 2/2] PCI: keystone: Remove the __init macro for the ks_pcie_host_init() callback Siddharth Vadapalli
2025-10-02 14:36   ` Bjorn Helgaas
2025-10-22 10:03     ` Siddharth Vadapalli
2025-09-29 16:25 ` [PATCH 0/2] PCI: Keystone: __init and IRQ Fixes Manivannan Sadhasivam

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