linux-kernel - Re: [PATCH 1/2] iommu/riscv: Free irq vectors on pci remove

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20241107-4570ae5b32599c6ff96b5241@orel>
Date: Thu, 7 Nov 2024 17:35:19 +0100
From: Andrew Jones <ajones@...tanamicro.com>
To: Tomasz Jeznach <tjeznach@...osinc.com>
Cc: iommu@...ts.linux.dev, linux-riscv@...ts.infradead.org, 
	linux-kernel@...r.kernel.org, joro@...tes.org, will@...nel.org, robin.murphy@....com, 
	paul.walmsley@...ive.com, palmer@...belt.com, aou@...s.berkeley.edu
Subject: Re: [PATCH 1/2] iommu/riscv: Free irq vectors on pci remove

On Wed, Nov 06, 2024 at 11:00:55AM -0800, Tomasz Jeznach wrote:
> On Wed, Nov 6, 2024 at 9:51 AM Andrew Jones <ajones@...tanamicro.com> wrote:
> >
> > riscv_iommu_pci_probe() calls pci_alloc_irq_vectors() which
> > states pci_free_irq_vectors() must be called on cleanup.
> >
> > Fixes: 68682e9578fb ("iommu/riscv: Add RISC-V IOMMU PCIe device driver")
> > Signed-off-by: Andrew Jones <ajones@...tanamicro.com>
> > ---
> >  drivers/iommu/riscv/iommu-pci.c | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/iommu/riscv/iommu-pci.c b/drivers/iommu/riscv/iommu-pci.c
> > index c7a89143014c..25a27e627a0e 100644
> > --- a/drivers/iommu/riscv/iommu-pci.c
> > +++ b/drivers/iommu/riscv/iommu-pci.c
> > @@ -99,6 +99,7 @@ static void riscv_iommu_pci_remove(struct pci_dev *pdev)
> >         struct riscv_iommu_device *iommu = dev_get_drvdata(&pdev->dev);
> >
> >         riscv_iommu_remove(iommu);
> > +       pci_free_irq_vectors(pdev);
> >  }
> >
> >  static const struct pci_device_id riscv_iommu_pci_tbl[] = {
> > --
> > 2.47.0
> >
> 
> Andrew, interrupt release call pci_free_irq_vectors() should already
> be called by device
> managed resources framework, with unwind action callback pcim_msi_release().
> 
> Callback is registered during vectors allocation:
>  pci_alloc_irq_vectors_affinity()
>   __pci_enable_msix_range()
>    pci_setup_msi_context()
>     pcim_setup_msi_release()
> 
> This driver enables device resources management with initial call to
> pcim_enable_device().

Ah, thank you for the education and inspiration to look closer at how this
works. It was easy to confirm with a WARN in pci_free_irq_vectors() and an
echo 1 to the IOMMU device's remove sysfs node.

> 
> Are there any conditions / testing sequence the release function is not called?

No, not that I know of, this was just my knee jerk decision to mimic the
call pairing I was doing on the platform side to the pci side. Sorry for
the noise, this patch can be dropped.

Thanks,
drew