Re: [PATCH 5/7] vDPA/ifcvf: irq request helpers for both shared and per_vq irq

["Michael S. Tsirkin" <mst@redhat.com>]

On Mon, Jan 10, 2022 at 01:18:49PM +0800, Zhu Lingshan wrote:
> This commit implements new irq request helpers:
> ifcvf_request_shared_vq_irq() for a shared irq, in this case,
> all virtqueues would share one irq/vector. This can help the
> device work on some platforms that can not provide enough
> MSIX vectors
> 
> ifcvf_request_per_vq_irq() for per vq irqs, in this case,
> every virtqueue has its own irq/vector
> 
> ifcvf_request_vq_irq() calls either of the above two, depends on
> the number of allocated vectors.
> 
> Signed-off-by: Zhu Lingshan <lingshan.zhu@intel.com>
> ---
>  drivers/vdpa/ifcvf/ifcvf_base.c |  9 -----
>  drivers/vdpa/ifcvf/ifcvf_main.c | 66 +++++++++++++++++++++++++++++++++
>  2 files changed, 66 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/vdpa/ifcvf/ifcvf_base.c b/drivers/vdpa/ifcvf/ifcvf_base.c
> index 696a41560eaa..fc496d10cf8d 100644
> --- a/drivers/vdpa/ifcvf/ifcvf_base.c
> +++ b/drivers/vdpa/ifcvf/ifcvf_base.c
> @@ -349,15 +349,6 @@ static int ifcvf_hw_enable(struct ifcvf_hw *hw)
>  		ifc_iowrite64_twopart(hw->vring[i].used, &cfg->queue_used_lo,
>  				     &cfg->queue_used_hi);
>  		ifc_iowrite16(hw->vring[i].size, &cfg->queue_size);
> -		ifc_iowrite16(i + IFCVF_MSI_QUEUE_OFF, &cfg->queue_msix_vector);
> -
> -		if (ifc_ioread16(&cfg->queue_msix_vector) ==
> -		    VIRTIO_MSI_NO_VECTOR) {
> -			IFCVF_ERR(ifcvf->pdev,
> -				  "No msix vector for queue %u\n", i);
> -			return -EINVAL;
> -		}
> -
>  		ifcvf_set_vq_state(hw, i, hw->vring[i].last_avail_idx);
>  		ifc_iowrite16(1, &cfg->queue_enable);
>  	}
> diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c b/drivers/vdpa/ifcvf/ifcvf_main.c
> index 19e1d1cd71a3..ce2fbc429fbe 100644
> --- a/drivers/vdpa/ifcvf/ifcvf_main.c
> +++ b/drivers/vdpa/ifcvf/ifcvf_main.c
> @@ -97,6 +97,72 @@ static int ifcvf_alloc_vectors(struct ifcvf_adapter *adapter)
>  	return ret;
>  }
>  
> +static int ifcvf_request_per_vq_irq(struct ifcvf_adapter *adapter)
> +{
> +	struct pci_dev *pdev = adapter->pdev;
> +	struct ifcvf_hw *vf = &adapter->vf;
> +	int i, vector, ret, irq;
> +
> +	for (i = 0; i < vf->nr_vring; i++) {
> +		snprintf(vf->vring[i].msix_name, 256, "ifcvf[%s]-%d\n", pci_name(pdev), i);
> +		vector = i;
> +		irq = pci_irq_vector(pdev, vector);
> +		ret = devm_request_irq(&pdev->dev, irq,
> +				       ifcvf_intr_handler, 0,
> +				       vf->vring[i].msix_name,
> +				       &vf->vring[i]);
> +		if (ret) {
> +			IFCVF_ERR(pdev, "Failed to request irq for vq %d\n", i);
> +			ifcvf_free_irq(adapter, i);
> +		} else {
> +			vf->vring[i].irq = irq;
> +			ifcvf_set_vq_vector(vf, i, vector);
> +		}
> +	}
> +
> +	return 0;
> +}
> +
> +static int ifcvf_request_shared_vq_irq(struct ifcvf_adapter *adapter)
> +{
> +	struct pci_dev *pdev = adapter->pdev;
> +	struct ifcvf_hw *vf = &adapter->vf;
> +	int i, vector, ret, irq;
> +
> +	vector = 0;
> +	irq = pci_irq_vector(pdev, vector);
> +	ret = devm_request_irq(&pdev->dev, irq,
> +			       ifcvf_shared_intr_handler, 0,
> +			       "ifcvf_shared_irq",
> +			       vf);
> +	if (ret) {
> +		IFCVF_ERR(pdev, "Failed to request shared irq for vf\n");
> +
> +		return ret;
> +	}
> +
> +	for (i = 0; i < vf->nr_vring; i++) {
> +		vf->vring[i].irq = irq;
> +		ifcvf_set_vq_vector(vf, i, vector);
> +	}
> +
> +	return 0;
> +
> +}
> +
> +static int ifcvf_request_vq_irq(struct ifcvf_adapter *adapter, u8 vector_per_vq)
> +{
> +	int ret;
> +
> +	if (vector_per_vq)
> +		ret = ifcvf_request_per_vq_irq(adapter);
> +	else
> +		ret = ifcvf_request_shared_vq_irq(adapter);
> +
> +	return ret;
> +}
> +
> +
>  static int ifcvf_request_irq(struct ifcvf_adapter *adapter)
>  {
>  	struct pci_dev *pdev = adapter->pdev;


this moves code from init function to static ones which
are never called. I guess until patch 7? You can't
split up patches like this, git bisect won't work if you do -
code needs to work after each patch is applied.

> -- 
> 2.27.0