Messages in this thread Patch in this message | | | From | Rusty Russell <> | Subject | Re: [PATCH] virtio_pci: fix capability format, comments | Date | Thu, 30 May 2013 11:03:38 +0930 |
| |
"Michael S. Tsirkin" <mst@redhat.com> writes:
> - queue size can actually be 0 which is not a power of 2
Actually, that points to a flaw in the code. When we shut down the queue, we should ideally reset it to what the device started with, rather than 0.
See below.
> - fix capability format. PCI spec says: > The layout of the information is vendor specific, except that the byte > immediately following the “Next” pointer in the capability structure is > defined to be a length field. > This length field provides the number of bytes in the capability > structure (including the ID and Next pointer bytes).
That part's definitely correct: applied.
Thanks, Rusty.
Subjet: virtio_pci: save the desired ringsize.
MST points out that 0 isn't a power of 2. This means we can't re-open a virtio device once we write 0 into the queue length.
We should restore the amount the device originally asked for.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
diff --git a/drivers/virtio/virtio_pci-common.h b/drivers/virtio/virtio_pci-common.h index ba1bf81..0e3143b 100644 --- a/drivers/virtio/virtio_pci-common.h +++ b/drivers/virtio/virtio_pci-common.h @@ -82,6 +82,9 @@ struct virtio_pci_vq_info { /* MSI-X vector (or none) */ unsigned msix_vector; + + /* What size did the device *want* this to be? */ + u16 desired_num; }; /* the notify function used when creating a virt queue */ diff --git a/drivers/virtio/virtio_pci.c b/drivers/virtio/virtio_pci.c index 0f0e3a6..8b35c2e 100644 --- a/drivers/virtio/virtio_pci.c +++ b/drivers/virtio/virtio_pci.c @@ -230,6 +230,7 @@ static struct virtqueue *setup_vq(struct virtio_pci_device *vp_dev, return ERR_PTR(-ENOMEM); info->msix_vector = msix_vec; + info->desired_num = num; /* get offset of notification word for this vq (shouldn't wrap) */ off = ioread16(&vp_dev->common->queue_notify_off); @@ -350,7 +351,7 @@ static void del_vq(struct virtqueue *vq) vring_del_virtqueue(vq); /* This is for our own benefit, not the device's! */ - iowrite16(0, &vp_dev->common->queue_size); + iowrite16(info->desired_num, &vp_dev->common->queue_size); iowrite64_twopart(0, &vp_dev->common->queue_desc); iowrite64_twopart(0, &vp_dev->common->queue_avail); iowrite64_twopart(0, &vp_dev->common->queue_used); -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |