| Message ID | 1346154857-12487-2-git-send-email-pbonzini@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 08/28/2012 07:54 PM, Paolo Bonzini wrote: > From: Jason Wang<jasowang@redhat.com> > > Instead of storing the queue index in transport-specific virtio structs, > this patch moves them to vring_virtqueue and introduces an helper to get > the value. This lets drivers simplify their management and tracing of > virtqueues. > > Signed-off-by: Jason Wang<jasowang@redhat.com> > Signed-off-by: Paolo Bonzini<pbonzini@redhat.com> > --- > I fixed the problems in Jason's v5 (posted at > http://permalink.gmane.org/gmane.linux.kernel.virtualization/15910) > and switched from virtio_set_queue_index to a new argument of > vring_new_virtqueue. This breaks at compile-time any virtio > transport that is not updated. Thanks Paolo. I'm fine with this patch. > > drivers/lguest/lguest_device.c | 2 +- > drivers/remoteproc/remoteproc_virtio.c | 2 +- > drivers/s390/kvm/kvm_virtio.c | 2 +- > drivers/virtio/virtio_mmio.c | 12 ++++-------- > drivers/virtio/virtio_pci.c | 13 +++++-------- > drivers/virtio/virtio_ring.c | 14 +++++++++++++- > include/linux/virtio.h | 2 ++ > include/linux/virtio_ring.h | 3 ++- > 8 files changed, 29 insertions(+), 21 deletions(-) > > diff --git a/drivers/lguest/lguest_device.c b/drivers/lguest/lguest_device.c > index 9e8388e..ccb7dfb 100644 > --- a/drivers/lguest/lguest_device.c > +++ b/drivers/lguest/lguest_device.c > @@ -296,7 +296,7 @@ static struct virtqueue *lg_find_vq(struct virtio_device *vdev, > * to 'true': the host just a(nother) SMP CPU, so we only need inter-cpu > * barriers. > */ > - vq = vring_new_virtqueue(lvq->config.num, LGUEST_VRING_ALIGN, vdev, > + vq = vring_new_virtqueue(index, lvq->config.num, LGUEST_VRING_ALIGN, vdev, > true, lvq->pages, lg_notify, callback, name); > if (!vq) { > err = -ENOMEM; > diff --git a/drivers/remoteproc/remoteproc_virtio.c b/drivers/remoteproc/remoteproc_virtio.c > index 3541b44..343c194 100644 > --- a/drivers/remoteproc/remoteproc_virtio.c > +++ b/drivers/remoteproc/remoteproc_virtio.c > @@ -103,7 +103,7 @@ static struct virtqueue *rp_find_vq(struct virtio_device *vdev, > * Create the new vq, and tell virtio we're not interested in > * the 'weak' smp barriers, since we're talking with a real device. > */ > - vq = vring_new_virtqueue(len, rvring->align, vdev, false, addr, > + vq = vring_new_virtqueue(id, len, rvring->align, vdev, false, addr, > rproc_virtio_notify, callback, name); > if (!vq) { > dev_err(dev, "vring_new_virtqueue %s failed\n", name); > diff --git a/drivers/s390/kvm/kvm_virtio.c b/drivers/s390/kvm/kvm_virtio.c > index 47cccd5..5565af2 100644 > --- a/drivers/s390/kvm/kvm_virtio.c > +++ b/drivers/s390/kvm/kvm_virtio.c > @@ -198,7 +198,7 @@ static struct virtqueue *kvm_find_vq(struct virtio_device *vdev, > if (err) > goto out; > > - vq = vring_new_virtqueue(config->num, KVM_S390_VIRTIO_RING_ALIGN, > + vq = vring_new_virtqueue(index, config->num, KVM_S390_VIRTIO_RING_ALIGN, > vdev, true, (void *) config->address, > kvm_notify, callback, name); > if (!vq) { > diff --git a/drivers/virtio/virtio_mmio.c b/drivers/virtio/virtio_mmio.c > index 453db0c..008bf58 100644 > --- a/drivers/virtio/virtio_mmio.c > +++ b/drivers/virtio/virtio_mmio.c > @@ -131,9 +131,6 @@ struct virtio_mmio_vq_info { > /* the number of entries in the queue */ > unsigned int num; > > - /* the index of the queue */ > - int queue_index; > - > /* the virtual address of the ring queue */ > void *queue; > > @@ -225,11 +222,10 @@ static void vm_reset(struct virtio_device *vdev) > static void vm_notify(struct virtqueue *vq) > { > struct virtio_mmio_device *vm_dev = to_virtio_mmio_device(vq->vdev); > - struct virtio_mmio_vq_info *info = vq->priv; > > /* We write the queue's selector into the notification register to > * signal the other end */ > - writel(info->queue_index, vm_dev->base + VIRTIO_MMIO_QUEUE_NOTIFY); > + writel(virtqueue_get_queue_index(vq), vm_dev->base + VIRTIO_MMIO_QUEUE_NOTIFY); > } > > /* Notify all virtqueues on an interrupt. */ > @@ -270,6 +266,7 @@ static void vm_del_vq(struct virtqueue *vq) > struct virtio_mmio_device *vm_dev = to_virtio_mmio_device(vq->vdev); > struct virtio_mmio_vq_info *info = vq->priv; > unsigned long flags, size; > + unsigned int index = virtqueue_get_queue_index(vq); > > spin_lock_irqsave(&vm_dev->lock, flags); > list_del(&info->node); > @@ -278,7 +275,7 @@ static void vm_del_vq(struct virtqueue *vq) > vring_del_virtqueue(vq); > > /* Select and deactivate the queue */ > - writel(info->queue_index, vm_dev->base + VIRTIO_MMIO_QUEUE_SEL); > + writel(index, vm_dev->base + VIRTIO_MMIO_QUEUE_SEL); > writel(0, vm_dev->base + VIRTIO_MMIO_QUEUE_PFN); > > size = PAGE_ALIGN(vring_size(info->num, VIRTIO_MMIO_VRING_ALIGN)); > @@ -324,7 +321,6 @@ static struct virtqueue *vm_setup_vq(struct virtio_device *vdev, unsigned index, > err = -ENOMEM; > goto error_kmalloc; > } > - info->queue_index = index; > > /* Allocate pages for the queue - start with a queue as big as > * possible (limited by maximum size allowed by device), drop down > @@ -356,7 +352,7 @@ static struct virtqueue *vm_setup_vq(struct virtio_device *vdev, unsigned index, > vm_dev->base + VIRTIO_MMIO_QUEUE_PFN); > > /* Create the vring */ > - vq = vring_new_virtqueue(info->num, VIRTIO_MMIO_VRING_ALIGN, vdev, > + vq = vring_new_virtqueue(index, info->num, VIRTIO_MMIO_VRING_ALIGN, vdev, > true, info->queue, vm_notify, callback, name); > if (!vq) { > err = -ENOMEM; > diff --git a/drivers/virtio/virtio_pci.c b/drivers/virtio/virtio_pci.c > index 2e03d41..d902464 100644 > --- a/drivers/virtio/virtio_pci.c > +++ b/drivers/virtio/virtio_pci.c > @@ -79,9 +79,6 @@ struct virtio_pci_vq_info > /* the number of entries in the queue */ > int num; > > - /* the index of the queue */ > - int queue_index; > - > /* the virtual address of the ring queue */ > void *queue; > > @@ -202,11 +199,11 @@ static void vp_reset(struct virtio_device *vdev) > static void vp_notify(struct virtqueue *vq) > { > struct virtio_pci_device *vp_dev = to_vp_device(vq->vdev); > - struct virtio_pci_vq_info *info = vq->priv; > > /* we write the queue's selector into the notification register to > * signal the other end */ > - iowrite16(info->queue_index, vp_dev->ioaddr + VIRTIO_PCI_QUEUE_NOTIFY); > + iowrite16(virtqueue_get_queue_index(vq), > + vp_dev->ioaddr + VIRTIO_PCI_QUEUE_NOTIFY); > } > > /* Handle a configuration change: Tell driver if it wants to know. */ > @@ -402,7 +399,6 @@ static struct virtqueue *setup_vq(struct virtio_device *vdev, unsigned index, > if (!info) > return ERR_PTR(-ENOMEM); > > - info->queue_index = index; > info->num = num; > info->msix_vector = msix_vec; > > @@ -418,7 +414,7 @@ static struct virtqueue *setup_vq(struct virtio_device *vdev, unsigned index, > vp_dev->ioaddr + VIRTIO_PCI_QUEUE_PFN); > > /* create the vring */ > - vq = vring_new_virtqueue(info->num, VIRTIO_PCI_VRING_ALIGN, vdev, > + vq = vring_new_virtqueue(index, info->num, VIRTIO_PCI_VRING_ALIGN, vdev, > true, info->queue, vp_notify, callback, name); > if (!vq) { > err = -ENOMEM; > @@ -467,7 +463,8 @@ static void vp_del_vq(struct virtqueue *vq) > list_del(&info->node); > spin_unlock_irqrestore(&vp_dev->lock, flags); > > - iowrite16(info->queue_index, vp_dev->ioaddr + VIRTIO_PCI_QUEUE_SEL); > + iowrite16(virtqueue_get_queue_index(vq), > + vp_dev->ioaddr + VIRTIO_PCI_QUEUE_SEL); > > if (vp_dev->msix_enabled) { > iowrite16(VIRTIO_MSI_NO_VECTOR, > diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c > index 5aa43c3..e639584 100644 > --- a/drivers/virtio/virtio_ring.c > +++ b/drivers/virtio/virtio_ring.c > @@ -106,6 +106,9 @@ struct vring_virtqueue > /* How to notify other side. FIXME: commonalize hcalls! */ > void (*notify)(struct virtqueue *vq); > > + /* Index of the queue */ > + int queue_index; > + > #ifdef DEBUG > /* They're supposed to lock for us. */ > unsigned int in_use; > @@ -171,6 +174,13 @@ static int vring_add_indirect(struct vring_virtqueue *vq, > return head; > } > > +int virtqueue_get_queue_index(struct virtqueue *_vq) > +{ > + struct vring_virtqueue *vq = to_vvq(_vq); > + return vq->queue_index; > +} > +EXPORT_SYMBOL_GPL(virtqueue_get_queue_index); > + > /** > * virtqueue_add_buf - expose buffer to other end > * @vq: the struct virtqueue we're talking about. > @@ -616,7 +626,8 @@ irqreturn_t vring_interrupt(int irq, void *_vq) > } > EXPORT_SYMBOL_GPL(vring_interrupt); > > -struct virtqueue *vring_new_virtqueue(unsigned int num, > +struct virtqueue *vring_new_virtqueue(unsigned int index, > + unsigned int num, > unsigned int vring_align, > struct virtio_device *vdev, > bool weak_barriers, > @@ -647,6 +658,7 @@ struct virtqueue *vring_new_virtqueue(unsigned int num, > vq->broken = false; > vq->last_used_idx = 0; > vq->num_added = 0; > + vq->queue_index = index; > list_add_tail(&vq->vq.list,&vdev->vqs); > #ifdef DEBUG > vq->in_use = false; > diff --git a/include/linux/virtio.h b/include/linux/virtio.h > index a1ba8bb..533b115 100644 > --- a/include/linux/virtio.h > +++ b/include/linux/virtio.h > @@ -50,6 +50,8 @@ void *virtqueue_detach_unused_buf(struct virtqueue *vq); > > unsigned int virtqueue_get_vring_size(struct virtqueue *vq); > > +int virtqueue_get_queue_index(struct virtqueue *vq); > + > /** > * virtio_device - representation of a device using virtio > * @index: unique position on the virtio bus > diff --git a/include/linux/virtio_ring.h b/include/linux/virtio_ring.h > index e338730..c2d793a 100644 > --- a/include/linux/virtio_ring.h > +++ b/include/linux/virtio_ring.h > @@ -165,7 +165,8 @@ static inline int vring_need_event(__u16 event_idx, __u16 new_idx, __u16 old) > struct virtio_device; > struct virtqueue; > > -struct virtqueue *vring_new_virtqueue(unsigned int num, > +struct virtqueue *vring_new_virtqueue(unsigned int index, > + unsigned int num, > unsigned int vring_align, > struct virtio_device *vdev, > bool weak_barriers, -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Paolo Bonzini <pbonzini@redhat.com> writes: > From: Jason Wang <jasowang@redhat.com> > > Instead of storing the queue index in transport-specific virtio structs, > this patch moves them to vring_virtqueue and introduces an helper to get > the value. This lets drivers simplify their management and tracing of > virtqueues. > > Signed-off-by: Jason Wang <jasowang@redhat.com> > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> Sorry for the delay, I was at Kernel Summit and am only now actually reading (vs skimming) my backlog. Putting it in vring_virtqueue rather than virtqueue seems weird, though. But I've applied as-is, we can clean up that later if we want (probably by merging the two structures, I'll have to think harder on that). Acked-by: Rusty Russell <rusty@rustcorp.com.au> Cheers, Rusty. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/lguest/lguest_device.c b/drivers/lguest/lguest_device.c index 9e8388e..ccb7dfb 100644 --- a/drivers/lguest/lguest_device.c +++ b/drivers/lguest/lguest_device.c @@ -296,7 +296,7 @@ static struct virtqueue *lg_find_vq(struct virtio_device *vdev, * to 'true': the host just a(nother) SMP CPU, so we only need inter-cpu * barriers. */ - vq = vring_new_virtqueue(lvq->config.num, LGUEST_VRING_ALIGN, vdev, + vq = vring_new_virtqueue(index, lvq->config.num, LGUEST_VRING_ALIGN, vdev, true, lvq->pages, lg_notify, callback, name); if (!vq) { err = -ENOMEM; diff --git a/drivers/remoteproc/remoteproc_virtio.c b/drivers/remoteproc/remoteproc_virtio.c index 3541b44..343c194 100644 --- a/drivers/remoteproc/remoteproc_virtio.c +++ b/drivers/remoteproc/remoteproc_virtio.c @@ -103,7 +103,7 @@ static struct virtqueue *rp_find_vq(struct virtio_device *vdev, * Create the new vq, and tell virtio we're not interested in * the 'weak' smp barriers, since we're talking with a real device. */ - vq = vring_new_virtqueue(len, rvring->align, vdev, false, addr, + vq = vring_new_virtqueue(id, len, rvring->align, vdev, false, addr, rproc_virtio_notify, callback, name); if (!vq) { dev_err(dev, "vring_new_virtqueue %s failed\n", name); diff --git a/drivers/s390/kvm/kvm_virtio.c b/drivers/s390/kvm/kvm_virtio.c index 47cccd5..5565af2 100644 --- a/drivers/s390/kvm/kvm_virtio.c +++ b/drivers/s390/kvm/kvm_virtio.c @@ -198,7 +198,7 @@ static struct virtqueue *kvm_find_vq(struct virtio_device *vdev, if (err) goto out; - vq = vring_new_virtqueue(config->num, KVM_S390_VIRTIO_RING_ALIGN, + vq = vring_new_virtqueue(index, config->num, KVM_S390_VIRTIO_RING_ALIGN, vdev, true, (void *) config->address, kvm_notify, callback, name); if (!vq) { diff --git a/drivers/virtio/virtio_mmio.c b/drivers/virtio/virtio_mmio.c index 453db0c..008bf58 100644 --- a/drivers/virtio/virtio_mmio.c +++ b/drivers/virtio/virtio_mmio.c @@ -131,9 +131,6 @@ struct virtio_mmio_vq_info { /* the number of entries in the queue */ unsigned int num; - /* the index of the queue */ - int queue_index; - /* the virtual address of the ring queue */ void *queue; @@ -225,11 +222,10 @@ static void vm_reset(struct virtio_device *vdev) static void vm_notify(struct virtqueue *vq) { struct virtio_mmio_device *vm_dev = to_virtio_mmio_device(vq->vdev); - struct virtio_mmio_vq_info *info = vq->priv; /* We write the queue's selector into the notification register to * signal the other end */ - writel(info->queue_index, vm_dev->base + VIRTIO_MMIO_QUEUE_NOTIFY); + writel(virtqueue_get_queue_index(vq), vm_dev->base + VIRTIO_MMIO_QUEUE_NOTIFY); } /* Notify all virtqueues on an interrupt. */ @@ -270,6 +266,7 @@ static void vm_del_vq(struct virtqueue *vq) struct virtio_mmio_device *vm_dev = to_virtio_mmio_device(vq->vdev); struct virtio_mmio_vq_info *info = vq->priv; unsigned long flags, size; + unsigned int index = virtqueue_get_queue_index(vq); spin_lock_irqsave(&vm_dev->lock, flags); list_del(&info->node); @@ -278,7 +275,7 @@ static void vm_del_vq(struct virtqueue *vq) vring_del_virtqueue(vq); /* Select and deactivate the queue */ - writel(info->queue_index, vm_dev->base + VIRTIO_MMIO_QUEUE_SEL); + writel(index, vm_dev->base + VIRTIO_MMIO_QUEUE_SEL); writel(0, vm_dev->base + VIRTIO_MMIO_QUEUE_PFN); size = PAGE_ALIGN(vring_size(info->num, VIRTIO_MMIO_VRING_ALIGN)); @@ -324,7 +321,6 @@ static struct virtqueue *vm_setup_vq(struct virtio_device *vdev, unsigned index, err = -ENOMEM; goto error_kmalloc; } - info->queue_index = index; /* Allocate pages for the queue - start with a queue as big as * possible (limited by maximum size allowed by device), drop down @@ -356,7 +352,7 @@ static struct virtqueue *vm_setup_vq(struct virtio_device *vdev, unsigned index, vm_dev->base + VIRTIO_MMIO_QUEUE_PFN); /* Create the vring */ - vq = vring_new_virtqueue(info->num, VIRTIO_MMIO_VRING_ALIGN, vdev, + vq = vring_new_virtqueue(index, info->num, VIRTIO_MMIO_VRING_ALIGN, vdev, true, info->queue, vm_notify, callback, name); if (!vq) { err = -ENOMEM; diff --git a/drivers/virtio/virtio_pci.c b/drivers/virtio/virtio_pci.c index 2e03d41..d902464 100644 --- a/drivers/virtio/virtio_pci.c +++ b/drivers/virtio/virtio_pci.c @@ -79,9 +79,6 @@ struct virtio_pci_vq_info /* the number of entries in the queue */ int num; - /* the index of the queue */ - int queue_index; - /* the virtual address of the ring queue */ void *queue; @@ -202,11 +199,11 @@ static void vp_reset(struct virtio_device *vdev) static void vp_notify(struct virtqueue *vq) { struct virtio_pci_device *vp_dev = to_vp_device(vq->vdev); - struct virtio_pci_vq_info *info = vq->priv; /* we write the queue's selector into the notification register to * signal the other end */ - iowrite16(info->queue_index, vp_dev->ioaddr + VIRTIO_PCI_QUEUE_NOTIFY); + iowrite16(virtqueue_get_queue_index(vq), + vp_dev->ioaddr + VIRTIO_PCI_QUEUE_NOTIFY); } /* Handle a configuration change: Tell driver if it wants to know. */ @@ -402,7 +399,6 @@ static struct virtqueue *setup_vq(struct virtio_device *vdev, unsigned index, if (!info) return ERR_PTR(-ENOMEM); - info->queue_index = index; info->num = num; info->msix_vector = msix_vec; @@ -418,7 +414,7 @@ static struct virtqueue *setup_vq(struct virtio_device *vdev, unsigned index, vp_dev->ioaddr + VIRTIO_PCI_QUEUE_PFN); /* create the vring */ - vq = vring_new_virtqueue(info->num, VIRTIO_PCI_VRING_ALIGN, vdev, + vq = vring_new_virtqueue(index, info->num, VIRTIO_PCI_VRING_ALIGN, vdev, true, info->queue, vp_notify, callback, name); if (!vq) { err = -ENOMEM; @@ -467,7 +463,8 @@ static void vp_del_vq(struct virtqueue *vq) list_del(&info->node); spin_unlock_irqrestore(&vp_dev->lock, flags); - iowrite16(info->queue_index, vp_dev->ioaddr + VIRTIO_PCI_QUEUE_SEL); + iowrite16(virtqueue_get_queue_index(vq), + vp_dev->ioaddr + VIRTIO_PCI_QUEUE_SEL); if (vp_dev->msix_enabled) { iowrite16(VIRTIO_MSI_NO_VECTOR, diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c index 5aa43c3..e639584 100644 --- a/drivers/virtio/virtio_ring.c +++ b/drivers/virtio/virtio_ring.c @@ -106,6 +106,9 @@ struct vring_virtqueue /* How to notify other side. FIXME: commonalize hcalls! */ void (*notify)(struct virtqueue *vq); + /* Index of the queue */ + int queue_index; + #ifdef DEBUG /* They're supposed to lock for us. */ unsigned int in_use; @@ -171,6 +174,13 @@ static int vring_add_indirect(struct vring_virtqueue *vq, return head; } +int virtqueue_get_queue_index(struct virtqueue *_vq) +{ + struct vring_virtqueue *vq = to_vvq(_vq); + return vq->queue_index; +} +EXPORT_SYMBOL_GPL(virtqueue_get_queue_index); + /** * virtqueue_add_buf - expose buffer to other end * @vq: the struct virtqueue we're talking about. @@ -616,7 +626,8 @@ irqreturn_t vring_interrupt(int irq, void *_vq) } EXPORT_SYMBOL_GPL(vring_interrupt); -struct virtqueue *vring_new_virtqueue(unsigned int num, +struct virtqueue *vring_new_virtqueue(unsigned int index, + unsigned int num, unsigned int vring_align, struct virtio_device *vdev, bool weak_barriers, @@ -647,6 +658,7 @@ struct virtqueue *vring_new_virtqueue(unsigned int num, vq->broken = false; vq->last_used_idx = 0; vq->num_added = 0; + vq->queue_index = index; list_add_tail(&vq->vq.list, &vdev->vqs); #ifdef DEBUG vq->in_use = false; diff --git a/include/linux/virtio.h b/include/linux/virtio.h index a1ba8bb..533b115 100644 --- a/include/linux/virtio.h +++ b/include/linux/virtio.h @@ -50,6 +50,8 @@ void *virtqueue_detach_unused_buf(struct virtqueue *vq); unsigned int virtqueue_get_vring_size(struct virtqueue *vq); +int virtqueue_get_queue_index(struct virtqueue *vq); + /** * virtio_device - representation of a device using virtio * @index: unique position on the virtio bus diff --git a/include/linux/virtio_ring.h b/include/linux/virtio_ring.h index e338730..c2d793a 100644 --- a/include/linux/virtio_ring.h +++ b/include/linux/virtio_ring.h @@ -165,7 +165,8 @@ static inline int vring_need_event(__u16 event_idx, __u16 new_idx, __u16 old) struct virtio_device; struct virtqueue; -struct virtqueue *vring_new_virtqueue(unsigned int num, +struct virtqueue *vring_new_virtqueue(unsigned int index, + unsigned int num, unsigned int vring_align, struct virtio_device *vdev, bool weak_barriers,