linux-kernel - Re: [PATCH 2/2] PCI: tegra: Allow building as a module

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <lk37wtb25pr2rj3zhct5udaykr7joqw2mpgtupjq33of2xhesi@rmdgucbzxmgz>
Date: Mon, 21 Apr 2025 14:24:28 +0530
From: Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>
To: Aaron Kling <webgeek1234@...il.com>
Cc: Thomas Gleixner <tglx@...utronix.de>, 
	Lorenzo Pieralisi <lpieralisi@...nel.org>, Krzysztof Wilczyński <kw@...ux.com>, 
	Rob Herring <robh@...nel.org>, Bjorn Helgaas <bhelgaas@...gle.com>, 
	Thierry Reding <thierry.reding@...il.com>, Jonathan Hunter <jonathanh@...dia.com>, 
	linux-kernel@...r.kernel.org, linux-pci@...r.kernel.org, linux-tegra@...r.kernel.org
Subject: Re: [PATCH 2/2] PCI: tegra: Allow building as a module

On Mon, Apr 21, 2025 at 03:09:42AM -0500, Aaron Kling wrote:
> On Mon, Apr 21, 2025 at 2:52 AM Manivannan Sadhasivam
> <manivannan.sadhasivam@...aro.org> wrote:
> >
> > On Sun, Apr 20, 2025 at 09:59:06PM -0500, Aaron Kling via B4 Relay wrote:
> > > From: Aaron Kling <webgeek1234@...il.com>
> > >
> > > The driver works fine as a module, so allow building as such.
> > >
> >
> > In the past, the former irqchip maintainer raised concerns for allowing the
> > irqchip drivers to be removed from the kernel. The concern was mostly (afaik)
> > due to not disposing all IRQs before removing the irq_domain.
> >
> > So Marek submitted a series [1] that added a new API for that. But that series
> > didn't progress further. So if you want to make this driver a module, you need
> > to do 2 things:
> >
> > 1. Make sure the cited series gets merged and this driver uses the new API.
> > 2. Get an Ack from Thomas (who is the only irqchip maintainer now).
> 
> Should this be a hard blocker for building this one driver as a
> module? I did a quick grep of drivers/pci/controller for irq_domain,
> then compared several of the hits to the Kconfig. And every single one
> is tristate. Tegra is by far not a unique offender here.
> 

Not 'unique', yes. But the situation is a bit worse atm. Some of the patches
(making the driver as a module) were merged in the past without addressing the
mapping issue.

Please take a look at the reply from Marc:
https://lkml.iu.edu/hypermail/linux/kernel/2207.2/08367.html

Even though Marc said that disposing IRQs is not enough to make sure there are
no dangling pointers of the IRQs in the client drivers, I'm inclined to atleast
allow modular drivers if they could dispose all the mappings with the new API.
This doesn't mean that I'm not cared about the potential issue, but the removing
of modules is always an 'experimental' feature in the kernel. So users should be
aware of what they are doing. Also, we have not seen any reported issues after
disposing the IRQs from the controller drivers. That also adds to my view on
this issue.

That being said, the safest option would be to get rid of the remove callback
and make the module modular. This will allow the driver to be built as a module
but never getting removed (make sure .suppress_bind_attrs is also set).

- Mani

> Sincerely,
> Aaron
> 
> >
> > - Mani
> >
> > [1] https://lore.kernel.org/linux-pci/20240715114854.4792-1-kabel@kernel.org
> >
> > > Signed-off-by: Aaron Kling <webgeek1234@...il.com>
> > > ---
> > >  drivers/pci/controller/Kconfig     | 2 +-
> > >  drivers/pci/controller/pci-tegra.c | 3 +++
> > >  2 files changed, 4 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/pci/controller/Kconfig b/drivers/pci/controller/Kconfig
> > > index 9800b768105402d6dd1ba4b134c2ec23da6e4201..a9164dd2eccaead5ae9348c24a5ad75fcb40f507 100644
> > > --- a/drivers/pci/controller/Kconfig
> > > +++ b/drivers/pci/controller/Kconfig
> > > @@ -224,7 +224,7 @@ config PCI_HYPERV_INTERFACE
> > >         driver.
> > >
> > >  config PCI_TEGRA
> > > -     bool "NVIDIA Tegra PCIe controller"
> > > +     tristate "NVIDIA Tegra PCIe controller"
> > >       depends on ARCH_TEGRA || COMPILE_TEST
> > >       depends on PCI_MSI
> > >       help
> > > diff --git a/drivers/pci/controller/pci-tegra.c b/drivers/pci/controller/pci-tegra.c
> > > index b3cdbc5927de3742161310610dc5dcb836f5dd69..c260842695f2e983ae48fd52b43f62dbb9fb5dd3 100644
> > > --- a/drivers/pci/controller/pci-tegra.c
> > > +++ b/drivers/pci/controller/pci-tegra.c
> > > @@ -2803,3 +2803,6 @@ static struct platform_driver tegra_pcie_driver = {
> > >       .remove = tegra_pcie_remove,
> > >  };
> > >  module_platform_driver(tegra_pcie_driver);
> > > +MODULE_AUTHOR("Thierry Reding <treding@...dia.com>");
> > > +MODULE_DESCRIPTION("NVIDIA PCI host controller driver");
> > > +MODULE_LICENSE("GPL");
> > >
> > > --
> > > 2.48.1
> > >
> > >
> >
> > --
> > மணிவண்ணன் சதாசிவம்

-- 
மணிவண்ணன் சதாசிவம்