On Wed, 19 Jan 2022 14:04:51 +0800, Jason Wang <jasowang@xxxxxxxxxx> wrote:
> 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".
OK.
>
> > +
> > 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.
OK
Thanks.
>
> 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
