1) Drop the FIXME reference to BIR, just use BAR.
2) Make queue addresses explicit: desc/avail/used.
3) Add an explicit number of queues.
4) Have explicit queue_enable, which can double as a method to disable
   all activity on a queue (write 0, read until 0).

I also noticed that the 64-bit queue_address was at offset 28 (0x1C),
which is unusual, so add more padding to take the first 64-bit value
to offset 32 (0x20).

Cc: H. Peter Anvin <h...@zytor.com>
Signed-off-by: Rusty Russell <ru...@rustcorp.com.au>
---
 drivers/virtio/virtio_pci.c     |   48 +++++++++++++++++++++++++++++++--------
 include/uapi/linux/virtio_pci.h |   11 +++++----
 2 files changed, 46 insertions(+), 13 deletions(-)

diff --git a/drivers/virtio/virtio_pci.c b/drivers/virtio/virtio_pci.c
index b86b99c..0169531 100644
--- a/drivers/virtio/virtio_pci.c
+++ b/drivers/virtio/virtio_pci.c
@@ -457,13 +457,14 @@ static struct virtqueue *setup_vq(struct virtio_device 
*vdev, unsigned index,
        u16 num;
        int err;
 
+       if (index >= ioread16(&vp_dev->common->num_queues))
+               return ERR_PTR(-ENOENT);
+
        /* Select the queue we're interested in */
        iowrite16(index, &vp_dev->common->queue_select);
 
-       switch (ioread64(&vp_dev->common->queue_address)) {
-       case 0xFFFFFFFFFFFFFFFFULL:
-               return ERR_PTR(-ENOENT);
-       case 0:
+       /* Sanity check */
+       switch (ioread64(&vp_dev->common->queue_desc)) {
                /* Uninitialized.  Excellent. */
                break;
        default:
@@ -522,9 +523,11 @@ static struct virtqueue *setup_vq(struct virtio_device 
*vdev, unsigned index,
        }
 
        /* Activate the queue. */
-       iowrite64(virt_to_phys(info->queue), &vp_dev->common->queue_address);
-       iowrite16(SMP_CACHE_BYTES, &vp_dev->common->queue_align);
        iowrite16(num, &vp_dev->common->queue_size);
+       iowrite64(virt_to_phys(vq->vring.desc), &vp_dev->common->queue_desc);
+       iowrite64(virt_to_phys(vq->vring.avail), &vp_dev->common->queue_avail);
+       iowrite64(virt_to_phys(vq->vring.used), &vp_dev->common->queue_used);
+       iowrite8(1, &vp_dev->common->queue_enable);
 
        return vq;
 
@@ -537,6 +540,29 @@ out_info:
        return ERR_PTR(err);
 }
 
+static void vp_vq_disable(struct virtio_pci_device *vp_dev,
+                         struct virtqueue *vq)
+{
+       unsigned long end;
+
+       /* Select the queue */
+       iowrite16(vq->index, &vp_dev->common->queue_select);
+
+       /* Disable it */
+       iowrite16(0, &vp_dev->common->queue_enable);
+
+       /* It's almost certainly synchronous, but just in case. */
+       end = jiffies + HZ/2;
+       while (ioread16(&vp_dev->common->queue_enable) != 0) {
+               if (time_after(jiffies, end)) {
+                       dev_warn(&vp_dev->pci_dev->dev,
+                                "virtio_pci: disable ignored\n");
+                       break;
+               }
+               cpu_relax();
+       }
+}
+
 static void vp_del_vq(struct virtqueue *vq)
 {
        struct virtio_pci_device *vp_dev = to_vp_device(vq->vdev);
@@ -547,7 +573,10 @@ static void vp_del_vq(struct virtqueue *vq)
        list_del(&info->node);
        spin_unlock_irqrestore(&vp_dev->lock, flags);
 
-       /* Select and deactivate the queue */
+       /* It should be quiescent, but disable first just in case. */
+       vp_vq_disable(vp_dev, vq);
+
+       /* Select the queue */
        iowrite16(vq->index, &vp_dev->common->queue_select);
 
        if (vp_dev->msix_enabled) {
@@ -560,9 +589,10 @@ static void vp_del_vq(struct virtqueue *vq)
        vring_del_virtqueue(vq);
 
        /* This is for our own benefit, not the device's! */
-       iowrite64(0, &vp_dev->common->queue_address);
        iowrite16(0, &vp_dev->common->queue_size);
-       iowrite16(0, &vp_dev->common->queue_align);
+       iowrite64(0, &vp_dev->common->queue_desc);
+       iowrite64(0, &vp_dev->common->queue_avail);
+       iowrite64(0, &vp_dev->common->queue_used);
 
        free_pages_exact(info->queue, size);
        kfree(info);
diff --git a/include/uapi/linux/virtio_pci.h b/include/uapi/linux/virtio_pci.h
index 0d12828..b334cd9 100644
--- a/include/uapi/linux/virtio_pci.h
+++ b/include/uapi/linux/virtio_pci.h
@@ -128,7 +128,6 @@ struct virtio_pci_cap {
        u8 cap_vndr;    /* Generic PCI field: PCI_CAP_ID_VNDR */
        u8 cap_next;    /* Generic PCI field: next ptr. */
        u8 cfg_type;    /* One of the VIRTIO_PCI_CAP_*_CFG. */
-/* FIXME: Should we use a bir, instead of raw bar number? */
        u8 bar;         /* Where to find it. */
        __le32 offset;  /* Offset within bar. */
        __le32 length;  /* Length. */
@@ -142,14 +141,18 @@ struct virtio_pci_common_cfg {
        __le32 guest_feature_select;    /* read-write */
        __le32 guest_feature;           /* read-only */
        __le16 msix_config;             /* read-write */
+       __le16 num_queues;              /* read-only */
        __u8 device_status;             /* read-write */
-       __u8 unused;
+       __u8 unused1;
+       __le16 unused2;
 
        /* About a specific virtqueue. */
        __le16 queue_select;    /* read-write */
-       __le16 queue_align;     /* read-write, power of 2. */
        __le16 queue_size;      /* read-write, power of 2. */
        __le16 queue_msix_vector;/* read-write */
-       __le64 queue_address;   /* read-write: 0xFFFFFFFFFFFFFFFF == DNE. */
+       __le16 queue_enable;    /* read-write */
+       __le64 queue_desc;      /* read-write */
+       __le64 queue_avail;     /* read-write */
+       __le64 queue_used;      /* read-write */
 };
 #endif /* _UAPI_LINUX_VIRTIO_PCI_H */
-- 
1.7.10.4

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Reply via email to