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

Andrew Jones
2024-11-07T18:01:59Z

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@ventanamicro.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@ventanamicro.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