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

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().

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

Thanks,
- Tomasz

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