On Wed, Jan 19, 2022 at 10:35 AM Xuan Zhuo <xuanzhuo@xxxxxxxxxxxxxxxxx> wrote:
>
> Add a vq_enable_vq_msix() function to enable a vq alone.
>
> Move irq's processing logic into vp_del_vq(), so that this function can
> handle a vq's del operation independently.
>
> In the subsequent patches that supports queue reset, I have the
> need to delete/enable a vq separately.
>
> Signed-off-by: Xuan Zhuo <xuanzhuo@xxxxxxxxxxxxxxxxx>
> ---
> drivers/virtio/virtio_pci_common.c | 86 +++++++++++++++++++-----------
> 1 file changed, 54 insertions(+), 32 deletions(-)
>
> diff --git a/drivers/virtio/virtio_pci_common.c b/drivers/virtio/virtio_pci_common.c
> index fdbde1db5ec5..5afe207ce28a 100644
> --- a/drivers/virtio/virtio_pci_common.c
> +++ b/drivers/virtio/virtio_pci_common.c
> @@ -248,6 +248,17 @@ static void vp_del_vq(struct virtqueue *vq)
> struct virtio_pci_vq_info *info = vp_dev->vqs[vq->index];
> unsigned long flags;
>
> + if (vp_dev->per_vq_vectors) {
> + int v = vp_dev->vqs[vq->index]->msix_vector;
> +
> + if (v != VIRTIO_MSI_NO_VECTOR) {
> + int irq = pci_irq_vector(vp_dev->pci_dev, v);
> +
> + irq_set_affinity_hint(irq, NULL);
> + free_irq(irq, vq);
> + }
> + }
> +
> spin_lock_irqsave(&vp_dev->lock, flags);
> list_del(&info->node);
> spin_unlock_irqrestore(&vp_dev->lock, flags);
> @@ -263,19 +274,9 @@ void vp_del_vqs(struct virtio_device *vdev)
> struct virtqueue *vq, *n;
> int i;
>
> - list_for_each_entry_safe(vq, n, &vdev->vqs, list) {
> - if (vp_dev->per_vq_vectors) {
> - int v = vp_dev->vqs[vq->index]->msix_vector;
> -
> - if (v != VIRTIO_MSI_NO_VECTOR) {
> - int irq = pci_irq_vector(vp_dev->pci_dev, v);
> -
> - irq_set_affinity_hint(irq, NULL);
> - free_irq(irq, vq);
> - }
> - }
> + list_for_each_entry_safe(vq, n, &vdev->vqs, list)
> vp_del_vq(vq);
> - }
I think we need a better subject of the patch and it's better to even
split the patch into two.
This part could be "move the per queue irq logic from vp_del_vqs to vp_del_vq".
> +
> vp_dev->per_vq_vectors = false;
>
> if (vp_dev->intx_enabled) {
> @@ -310,6 +311,40 @@ void vp_del_vqs(struct virtio_device *vdev)
> vp_dev->vqs = NULL;
> }
>
> +static struct virtqueue *vp_enable_vq_msix(struct virtio_device *vdev,
> + int queue_index,
> + vq_callback_t *callback,
> + const char * const name,
> + bool ctx,
> + u16 msix_vec)
> +{
> + struct virtio_pci_device *vp_dev = to_vp_device(vdev);
> + struct virtqueue *vq;
> + int err;
> +
> + vq = vp_setup_vq(vdev, queue_index, callback, name, ctx, msix_vec);
> + if (IS_ERR(vq))
> + return vq;
> +
> + if (!vp_dev->per_vq_vectors || msix_vec == VIRTIO_MSI_NO_VECTOR)
> + return vq;
> +
> + /* allocate per-vq irq if available and necessary */
> + snprintf(vp_dev->msix_names[msix_vec],
> + sizeof *vp_dev->msix_names,
> + "%s-%s", dev_name(&vp_dev->vdev.dev), name);
> +
> + err = request_irq(pci_irq_vector(vp_dev->pci_dev, msix_vec),
> + vring_interrupt, IRQF_NO_AUTOEN,
> + vp_dev->msix_names[msix_vec], vq);
> + if (err) {
> + vp_del_vq(vq);
> + return ERR_PTR(err);
> + }
> +
> + return vq;
> +}
And this deserves a separate patch as well.
Thanks
> +
> static int vp_find_vqs_msix(struct virtio_device *vdev, unsigned nvqs,
> struct virtqueue *vqs[], vq_callback_t *callbacks[],
> const char * const names[], bool per_vq_vectors,
> @@ -319,6 +354,7 @@ static int vp_find_vqs_msix(struct virtio_device *vdev, unsigned nvqs,
> struct virtio_pci_device *vp_dev = to_vp_device(vdev);
> u16 msix_vec;
> int i, err, nvectors, allocated_vectors, queue_idx = 0;
> + struct virtqueue *vq;
>
> vp_dev->vqs = kcalloc(nvqs, sizeof(*vp_dev->vqs), GFP_KERNEL);
> if (!vp_dev->vqs)
> @@ -354,28 +390,14 @@ static int vp_find_vqs_msix(struct virtio_device *vdev, unsigned nvqs,
> msix_vec = allocated_vectors++;
> else
> msix_vec = VP_MSIX_VQ_VECTOR;
> - vqs[i] = vp_setup_vq(vdev, queue_idx++, callbacks[i], names[i],
> - ctx ? ctx[i] : false,
> - msix_vec);
> - if (IS_ERR(vqs[i])) {
> - err = PTR_ERR(vqs[i]);
> - goto error_find;
> - }
>
> - if (!vp_dev->per_vq_vectors || msix_vec == VIRTIO_MSI_NO_VECTOR)
> - continue;
> -
> - /* allocate per-vq irq if available and necessary */
> - snprintf(vp_dev->msix_names[msix_vec],
> - sizeof *vp_dev->msix_names,
> - "%s-%s",
> - dev_name(&vp_dev->vdev.dev), names[i]);
> - err = request_irq(pci_irq_vector(vp_dev->pci_dev, msix_vec),
> - vring_interrupt, IRQF_NO_AUTOEN,
> - vp_dev->msix_names[msix_vec],
> - vqs[i]);
> - if (err)
> + vq = vp_enable_vq_msix(vdev, queue_idx++, callbacks[i],
> + names[i], ctx ? ctx[i] : false, msix_vec);
> + if (IS_ERR(vq)) {
> + err = PTR_ERR(vq);
> goto error_find;
> + }
> + vqs[i] = vq;
> }
> return 0;
>
> --
> 2.31.0
>
_______________________________________________
Virtualization mailing list
Virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
