Re: [PATCH 4/6] vfio_pci: use pci_irq_allocate_vectors

[Alex Williamson]

On Sun, 11 Sep 2016 15:31:26 +0200
Christoph Hellwig <hch@xxxxxx> wrote:

> Simply the interrupt setup by using the new PCI layer helpers.
> 
> Signed-off-by: Christoph Hellwig <hch@xxxxxx>
> ---
>  drivers/vfio/pci/vfio_pci_intrs.c   | 45 +++++++++----------------------------
>  drivers/vfio/pci/vfio_pci_private.h |  1 -
>  2 files changed, 10 insertions(+), 36 deletions(-)

Sorry for the delay, slipped by me.  Overall a really nice cleanup.
One tiny nit, the commit log mis-names the function as
pci_irq_allocate_vectors instead of pci_alloc_irq_vectors.  With that,

Acked-by: Alex Williamson <alex.williamson@xxxxxxxxxx>

Let me know if you're wanting me to pull this through my tree, I'm
assuming not.  Thanks,

Alex
 
> diff --git a/drivers/vfio/pci/vfio_pci_intrs.c b/drivers/vfio/pci/vfio_pci_intrs.c
> index 152b438..a1d283e 100644
> --- a/drivers/vfio/pci/vfio_pci_intrs.c
> +++ b/drivers/vfio/pci/vfio_pci_intrs.c
> @@ -250,6 +250,7 @@ static irqreturn_t vfio_msihandler(int irq, void *arg)
>  static int vfio_msi_enable(struct vfio_pci_device *vdev, int nvec, bool msix)
>  {
>  	struct pci_dev *pdev = vdev->pdev;
> +	unsigned int flag = msix ? PCI_IRQ_MSIX : PCI_IRQ_MSI;
>  	int ret;
>  
>  	if (!is_irq_none(vdev))
> @@ -259,35 +260,13 @@ static int vfio_msi_enable(struct vfio_pci_device *vdev, int nvec, bool msix)
>  	if (!vdev->ctx)
>  		return -ENOMEM;
>  
> -	if (msix) {
> -		int i;
> -
> -		vdev->msix = kzalloc(nvec * sizeof(struct msix_entry),
> -				     GFP_KERNEL);
> -		if (!vdev->msix) {
> -			kfree(vdev->ctx);
> -			return -ENOMEM;
> -		}
> -
> -		for (i = 0; i < nvec; i++)
> -			vdev->msix[i].entry = i;
> -
> -		ret = pci_enable_msix_range(pdev, vdev->msix, 1, nvec);
> -		if (ret < nvec) {
> -			if (ret > 0)
> -				pci_disable_msix(pdev);
> -			kfree(vdev->msix);
> -			kfree(vdev->ctx);
> -			return ret;
> -		}
> -	} else {
> -		ret = pci_enable_msi_range(pdev, 1, nvec);
> -		if (ret < nvec) {
> -			if (ret > 0)
> -				pci_disable_msi(pdev);
> -			kfree(vdev->ctx);
> -			return ret;
> -		}
> +	/* return the number of supported vectors if we can't get all: */
> +	ret = pci_alloc_irq_vectors(pdev, 1, nvec, flag);
> +	if (ret < nvec) {
> +		if (ret > 0)
> +			pci_free_irq_vectors(pdev);
> +		kfree(vdev->ctx);
> +		return ret;
>  	}
>  
>  	vdev->num_ctx = nvec;
> @@ -315,7 +294,7 @@ static int vfio_msi_set_vector_signal(struct vfio_pci_device *vdev,
>  	if (vector < 0 || vector >= vdev->num_ctx)
>  		return -EINVAL;
>  
> -	irq = msix ? vdev->msix[vector].vector : pdev->irq + vector;
> +	irq = pci_irq_vector(pdev, vector);
>  
>  	if (vdev->ctx[vector].trigger) {
>  		free_irq(irq, vdev->ctx[vector].trigger);
> @@ -408,11 +387,7 @@ static void vfio_msi_disable(struct vfio_pci_device *vdev, bool msix)
>  
>  	vfio_msi_set_block(vdev, 0, vdev->num_ctx, NULL, msix);
>  
> -	if (msix) {
> -		pci_disable_msix(vdev->pdev);
> -		kfree(vdev->msix);
> -	} else
> -		pci_disable_msi(pdev);
> +	pci_free_irq_vectors(pdev);
>  
>  	vdev->irq_type = VFIO_PCI_NUM_IRQS;
>  	vdev->num_ctx = 0;
> diff --git a/drivers/vfio/pci/vfio_pci_private.h b/drivers/vfio/pci/vfio_pci_private.h
> index 2128de8..f561ac1 100644
> --- a/drivers/vfio/pci/vfio_pci_private.h
> +++ b/drivers/vfio/pci/vfio_pci_private.h
> @@ -72,7 +72,6 @@ struct vfio_pci_device {
>  	struct perm_bits	*msi_perm;
>  	spinlock_t		irqlock;
>  	struct mutex		igate;
> -	struct msix_entry	*msix;
>  	struct vfio_pci_irq_ctx	*ctx;
>  	int			num_ctx;
>  	int			irq_type;