Merge branch 'linus' into x86/urgent, to merge dependent patch
[firefly-linux-kernel-4.4.55.git] / tools / lguest / lguest.c
index 990671e61f87ad5e6d9dd60d6cb995a729d8e420..e44052483ed933bbbf8f09cd91a84d1ef1790835 100644 (file)
@@ -66,6 +66,7 @@ typedef uint8_t u8;
 #define VIRTIO_CONFIG_NO_LEGACY
 #define VIRTIO_PCI_NO_LEGACY
 #define VIRTIO_BLK_NO_LEGACY
+#define VIRTIO_NET_NO_LEGACY
 
 /* Use in-kernel ones, which defines VIRTIO_F_VERSION_1 */
 #include "../../include/uapi/linux/virtio_config.h"
@@ -170,6 +171,9 @@ struct device {
        /* Is it operational */
        bool running;
 
+       /* Has it written FEATURES_OK but not re-checked it? */
+       bool wrote_features_ok;
+
        /* PCI configuration */
        union {
                struct pci_config config;
@@ -197,6 +201,9 @@ struct virtqueue {
        /* Which device owns me. */
        struct device *dev;
 
+       /* Name for printing errors. */
+       const char *name;
+
        /* The actual ring of buffers. */
        struct vring vring;
 
@@ -246,6 +253,16 @@ static struct termios orig_term;
 #define le32_to_cpu(v32) (v32)
 #define le64_to_cpu(v64) (v64)
 
+/*
+ * A real device would ignore weird/non-compliant driver behaviour.  We
+ * stop and flag it, to help debugging Linux problems.
+ */
+#define bad_driver(d, fmt, ...) \
+       errx(1, "%s: bad driver: " fmt, (d)->name, ## __VA_ARGS__)
+#define bad_driver_vq(vq, fmt, ...)                           \
+       errx(1, "%s vq %s: bad driver: " fmt, (vq)->dev->name, \
+            vq->name, ## __VA_ARGS__)
+
 /* Is this iovec empty? */
 static bool iov_empty(const struct iovec iov[], unsigned int num_iov)
 {
@@ -258,7 +275,8 @@ static bool iov_empty(const struct iovec iov[], unsigned int num_iov)
 }
 
 /* Take len bytes from the front of this iovec. */
-static void iov_consume(struct iovec iov[], unsigned num_iov,
+static void iov_consume(struct device *d,
+                       struct iovec iov[], unsigned num_iov,
                        void *dest, unsigned len)
 {
        unsigned int i;
@@ -276,7 +294,7 @@ static void iov_consume(struct iovec iov[], unsigned num_iov,
                len -= used;
        }
        if (len != 0)
-               errx(1, "iovec too short!");
+               bad_driver(d, "iovec too short!");
 }
 
 /*L:100
@@ -612,7 +630,8 @@ static void tell_kernel(unsigned long start)
  * we have a convenient routine which checks it and exits with an error message
  * if something funny is going on:
  */
-static void *_check_pointer(unsigned long addr, unsigned int size,
+static void *_check_pointer(struct device *d,
+                           unsigned long addr, unsigned int size,
                            unsigned int line)
 {
        /*
@@ -620,7 +639,8 @@ static void *_check_pointer(unsigned long addr, unsigned int size,
         * or addr + size wraps around.
         */
        if ((addr + size) > guest_limit || (addr + size) < addr)
-               errx(1, "%s:%i: Invalid address %#lx", __FILE__, line, addr);
+               bad_driver(d, "%s:%i: Invalid address %#lx",
+                          __FILE__, line, addr);
        /*
         * We return a pointer for the caller's convenience, now we know it's
         * safe to use.
@@ -628,14 +648,14 @@ static void *_check_pointer(unsigned long addr, unsigned int size,
        return from_guest_phys(addr);
 }
 /* A macro which transparently hands the line number to the real function. */
-#define check_pointer(addr,size) _check_pointer(addr, size, __LINE__)
+#define check_pointer(d,addr,size) _check_pointer(d, addr, size, __LINE__)
 
 /*
  * Each buffer in the virtqueues is actually a chain of descriptors.  This
  * function returns the next descriptor in the chain, or vq->vring.num if we're
  * at the end.
  */
-static unsigned next_desc(struct vring_desc *desc,
+static unsigned next_desc(struct device *d, struct vring_desc *desc,
                          unsigned int i, unsigned int max)
 {
        unsigned int next;
@@ -650,7 +670,7 @@ static unsigned next_desc(struct vring_desc *desc,
        wmb();
 
        if (next >= max)
-               errx(1, "Desc next is %u", next);
+               bad_driver(d, "Desc next is %u", next);
 
        return next;
 }
@@ -668,7 +688,25 @@ static void trigger_irq(struct virtqueue *vq)
                return;
        vq->pending_used = 0;
 
-       /* If they don't want an interrupt, don't send one... */
+       /*
+        * 2.4.7.1:
+        *
+        *  If the VIRTIO_F_EVENT_IDX feature bit is not negotiated:
+        *    The driver MUST set flags to 0 or 1. 
+        */
+       if (vq->vring.avail->flags > 1)
+               bad_driver_vq(vq, "avail->flags = %u\n", vq->vring.avail->flags);
+
+       /*
+        * 2.4.7.2:
+        *
+        *  If the VIRTIO_F_EVENT_IDX feature bit is not negotiated:
+        *
+        *     - The device MUST ignore the used_event value.
+        *     - After the device writes a descriptor index into the used ring:
+        *         - If flags is 1, the device SHOULD NOT send an interrupt.
+        *         - If flags is 0, the device MUST send an interrupt.
+        */
        if (vq->vring.avail->flags & VRING_AVAIL_F_NO_INTERRUPT) {
                return;
        }
@@ -703,6 +741,14 @@ static unsigned wait_for_vq_desc(struct virtqueue *vq,
        struct vring_desc *desc;
        u16 last_avail = lg_last_avail(vq);
 
+       /*
+        * 2.4.7.1:
+        *
+        *   The driver MUST handle spurious interrupts from the device.
+        *
+        * That's why this is a while loop.
+        */
+
        /* There's nothing available? */
        while (last_avail == vq->vring.avail->idx) {
                u64 event;
@@ -736,8 +782,8 @@ static unsigned wait_for_vq_desc(struct virtqueue *vq,
 
        /* Check it isn't doing very strange things with descriptor numbers. */
        if ((u16)(vq->vring.avail->idx - last_avail) > vq->vring.num)
-               errx(1, "Guest moved used index from %u to %u",
-                    last_avail, vq->vring.avail->idx);
+               bad_driver_vq(vq, "Guest moved used index from %u to %u",
+                             last_avail, vq->vring.avail->idx);
 
        /* 
         * Make sure we read the descriptor number *after* we read the ring
@@ -754,7 +800,7 @@ static unsigned wait_for_vq_desc(struct virtqueue *vq,
 
        /* If their number is silly, that's a fatal mistake. */
        if (head >= vq->vring.num)
-               errx(1, "Guest says index %u is available", head);
+               bad_driver_vq(vq, "Guest says index %u is available", head);
 
        /* When we start there are none of either input nor output. */
        *out_num = *in_num = 0;
@@ -776,18 +822,66 @@ static unsigned wait_for_vq_desc(struct virtqueue *vq,
                 * descriptor chain.
                 */
                if (desc[i].flags & VRING_DESC_F_INDIRECT) {
+                       /* 2.4.5.3.1:
+                        *
+                        *  The driver MUST NOT set the VIRTQ_DESC_F_INDIRECT
+                        *  flag unless the VIRTIO_F_INDIRECT_DESC feature was
+                        *  negotiated.
+                        */
+                       if (!(vq->dev->features_accepted &
+                             (1<<VIRTIO_RING_F_INDIRECT_DESC)))
+                               bad_driver_vq(vq, "vq indirect not negotiated");
+
+                       /*
+                        * 2.4.5.3.1:
+                        *
+                        *   The driver MUST NOT set the VIRTQ_DESC_F_INDIRECT
+                        *   flag within an indirect descriptor (ie. only one
+                        *   table per descriptor).
+                        */
+                       if (desc != vq->vring.desc)
+                               bad_driver_vq(vq, "Indirect within indirect");
+
+                       /*
+                        * Proposed update VIRTIO-134 spells this out:
+                        *
+                        *   A driver MUST NOT set both VIRTQ_DESC_F_INDIRECT
+                        *   and VIRTQ_DESC_F_NEXT in flags.
+                        */
+                       if (desc[i].flags & VRING_DESC_F_NEXT)
+                               bad_driver_vq(vq, "indirect and next together");
+
                        if (desc[i].len % sizeof(struct vring_desc))
-                               errx(1, "Invalid size for indirect buffer table");
+                               bad_driver_vq(vq,
+                                             "Invalid size for indirect table");
+                       /*
+                        * 2.4.5.3.2:
+                        *
+                        *  The device MUST ignore the write-only flag
+                        *  (flags&VIRTQ_DESC_F_WRITE) in the descriptor that
+                        *  refers to an indirect table.
+                        *
+                        * We ignore it here: :)
+                        */
 
                        max = desc[i].len / sizeof(struct vring_desc);
-                       desc = check_pointer(desc[i].addr, desc[i].len);
+                       desc = check_pointer(vq->dev, desc[i].addr, desc[i].len);
                        i = 0;
+
+                       /* 2.4.5.3.1:
+                        *
+                        *  A driver MUST NOT create a descriptor chain longer
+                        *  than the Queue Size of the device.
+                        */
+                       if (max > vq->pci_config.queue_size)
+                               bad_driver_vq(vq,
+                                             "indirect has too many entries");
                }
 
                /* Grab the first descriptor, and check it's OK. */
                iov[*out_num + *in_num].iov_len = desc[i].len;
                iov[*out_num + *in_num].iov_base
-                       = check_pointer(desc[i].addr, desc[i].len);
+                       = check_pointer(vq->dev, desc[i].addr, desc[i].len);
                /* If this is an input descriptor, increment that count. */
                if (desc[i].flags & VRING_DESC_F_WRITE)
                        (*in_num)++;
@@ -797,14 +891,15 @@ static unsigned wait_for_vq_desc(struct virtqueue *vq,
                         * to come before any input descriptors.
                         */
                        if (*in_num)
-                               errx(1, "Descriptor has out after in");
+                               bad_driver_vq(vq,
+                                             "Descriptor has out after in");
                        (*out_num)++;
                }
 
                /* If we've got too many, that implies a descriptor loop. */
                if (*out_num + *in_num > max)
-                       errx(1, "Looped descriptor");
-       } while ((i = next_desc(desc, i, max)) != max);
+                       bad_driver_vq(vq, "Looped descriptor");
+       } while ((i = next_desc(vq->dev, desc, i, max)) != max);
 
        return head;
 }
@@ -861,7 +956,7 @@ static void console_input(struct virtqueue *vq)
        /* Make sure there's a descriptor available. */
        head = wait_for_vq_desc(vq, iov, &out_num, &in_num);
        if (out_num)
-               errx(1, "Output buffers in console in queue?");
+               bad_driver_vq(vq, "Output buffers in console in queue?");
 
        /* Read into it.  This is where we usually wait. */
        len = readv(STDIN_FILENO, iov, in_num);
@@ -914,7 +1009,7 @@ static void console_output(struct virtqueue *vq)
        /* We usually wait in here, for the Guest to give us something. */
        head = wait_for_vq_desc(vq, iov, &out, &in);
        if (in)
-               errx(1, "Input buffers in console output queue?");
+               bad_driver_vq(vq, "Input buffers in console output queue?");
 
        /* writev can return a partial write, so we loop here. */
        while (!iov_empty(iov, out)) {
@@ -923,7 +1018,7 @@ static void console_output(struct virtqueue *vq)
                        warn("Write to stdout gave %i (%d)", len, errno);
                        break;
                }
-               iov_consume(iov, out, NULL, len);
+               iov_consume(vq->dev, iov, out, NULL, len);
        }
 
        /*
@@ -952,7 +1047,7 @@ static void net_output(struct virtqueue *vq)
        /* We usually wait in here for the Guest to give us a packet. */
        head = wait_for_vq_desc(vq, iov, &out, &in);
        if (in)
-               errx(1, "Input buffers in net output queue?");
+               bad_driver_vq(vq, "Input buffers in net output queue?");
        /*
         * Send the whole thing through to /dev/net/tun.  It expects the exact
         * same format: what a coincidence!
@@ -1000,7 +1095,7 @@ static void net_input(struct virtqueue *vq)
         */
        head = wait_for_vq_desc(vq, iov, &out, &in);
        if (out)
-               errx(1, "Output buffers in net input queue?");
+               bad_driver_vq(vq, "Output buffers in net input queue?");
 
        /*
         * If it looks like we'll block reading from the tun device, send them
@@ -1082,6 +1177,7 @@ static void reset_device(struct device *dev)
                }
        }
        dev->running = false;
+       dev->wrote_features_ok = false;
 
        /* Now we care if threads die. */
        signal(SIGCHLD, (void *)kill_launcher);
@@ -1382,7 +1478,8 @@ static void pci_data_ioread(u16 port, u32 mask, u32 *val)
                 */
                /* Must be bar 0 */
                if (!valid_bar_access(d, &d->config.cfg_access))
-                       errx(1, "Invalid cfg_access to bar%u, offset %u len %u",
+                       bad_driver(d,
+                            "Invalid cfg_access to bar%u, offset %u len %u",
                             d->config.cfg_access.cap.bar,
                             d->config.cfg_access.cap.offset,
                             d->config.cfg_access.cap.length);
@@ -1688,35 +1785,67 @@ static void restore_vq_config(struct virtio_pci_common_cfg *cfg,
 }
 
 /*
+ * 4.1.4.3.2:
+ *
+ *  The driver MUST configure the other virtqueue fields before
+ *  enabling the virtqueue with queue_enable.
+ *
  * When they enable the virtqueue, we check that their setup is valid.
  */
-static void enable_virtqueue(struct device *d, struct virtqueue *vq)
+static void check_virtqueue(struct device *d, struct virtqueue *vq)
 {
-       /*
-        * Create stack for thread.  Since the stack grows upwards, we point
-        * the stack pointer to the end of this region.
-        */
-       char *stack = malloc(32768);
-
        /* Because lguest is 32 bit, all the descriptor high bits must be 0 */
        if (vq->pci_config.queue_desc_hi
            || vq->pci_config.queue_avail_hi
            || vq->pci_config.queue_used_hi)
-               errx(1, "%s: invalid 64-bit queue address", d->name);
+               bad_driver_vq(vq, "invalid 64-bit queue address");
+
+       /*
+        * 2.4.1:
+        *
+        *  The driver MUST ensure that the physical address of the first byte
+        *  of each virtqueue part is a multiple of the specified alignment
+        *  value in the above table.
+        */
+       if (vq->pci_config.queue_desc_lo % 16
+           || vq->pci_config.queue_avail_lo % 2
+           || vq->pci_config.queue_used_lo % 4)
+               bad_driver_vq(vq, "invalid alignment in queue addresses");
 
        /* Initialize the virtqueue and check they're all in range. */
        vq->vring.num = vq->pci_config.queue_size;
-       vq->vring.desc = check_pointer(vq->pci_config.queue_desc_lo,
+       vq->vring.desc = check_pointer(vq->dev,
+                                      vq->pci_config.queue_desc_lo,
                                       sizeof(*vq->vring.desc) * vq->vring.num);
-       vq->vring.avail = check_pointer(vq->pci_config.queue_avail_lo,
+       vq->vring.avail = check_pointer(vq->dev,
+                                       vq->pci_config.queue_avail_lo,
                                        sizeof(*vq->vring.avail)
                                        + (sizeof(vq->vring.avail->ring[0])
                                           * vq->vring.num));
-       vq->vring.used = check_pointer(vq->pci_config.queue_used_lo,
+       vq->vring.used = check_pointer(vq->dev,
+                                      vq->pci_config.queue_used_lo,
                                       sizeof(*vq->vring.used)
                                       + (sizeof(vq->vring.used->ring[0])
                                          * vq->vring.num));
 
+       /*
+        * 2.4.9.1:
+        *
+        *   The driver MUST initialize flags in the used ring to 0
+        *   when allocating the used ring.
+        */
+       if (vq->vring.used->flags != 0)
+               bad_driver_vq(vq, "invalid initial used.flags %#x",
+                             vq->vring.used->flags);
+}
+
+static void start_virtqueue(struct virtqueue *vq)
+{
+       /*
+        * Create stack for thread.  Since the stack grows upwards, we point
+        * the stack pointer to the end of this region.
+        */
+       char *stack = malloc(32768);
 
        /* Create a zero-initialized eventfd. */
        vq->eventfd = eventfd(0, 0);
@@ -1732,6 +1861,16 @@ static void enable_virtqueue(struct device *d, struct virtqueue *vq)
                err(1, "Creating clone");
 }
 
+static void start_virtqueues(struct device *d)
+{
+       struct virtqueue *vq;
+
+       for (vq = d->vq; vq; vq = vq->next) {
+               if (vq->pci_config.queue_enable)
+                       start_virtqueue(vq);
+       }
+}
+
 static void emulate_mmio_write(struct device *d, u32 off, u32 val, u32 mask)
 {
        struct virtqueue *vq;
@@ -1751,12 +1890,11 @@ static void emulate_mmio_write(struct device *d, u32 off, u32 val, u32 mask)
                        d->mmio->cfg.device_feature = (d->features >> 32);
                else
                        d->mmio->cfg.device_feature = 0;
-               goto write_through32;
+               goto feature_write_through32;
        case offsetof(struct virtio_pci_mmio, cfg.guest_feature_select):
                if (val > 1)
-                       errx(1, "%s: Unexpected driver select %u",
-                            d->name, val);
-               goto write_through32;
+                       bad_driver(d, "Unexpected driver select %u", val);
+               goto feature_write_through32;
        case offsetof(struct virtio_pci_mmio, cfg.guest_feature):
                if (d->mmio->cfg.guest_feature_select == 0) {
                        d->features_accepted &= ~((u64)0xFFFFFFFF);
@@ -1766,11 +1904,19 @@ static void emulate_mmio_write(struct device *d, u32 off, u32 val, u32 mask)
                        d->features_accepted &= 0xFFFFFFFF;
                        d->features_accepted |= ((u64)val) << 32;
                }
+               /*
+                * 2.2.1:
+                *
+                *   The driver MUST NOT accept a feature which the device did
+                *   not offer
+                */
                if (d->features_accepted & ~d->features)
-                       errx(1, "%s: over-accepted features %#llx of %#llx",
-                            d->name, d->features_accepted, d->features);
-               goto write_through32;
-       case offsetof(struct virtio_pci_mmio, cfg.device_status):
+                       bad_driver(d, "over-accepted features %#llx of %#llx",
+                                  d->features_accepted, d->features);
+               goto feature_write_through32;
+       case offsetof(struct virtio_pci_mmio, cfg.device_status): {
+               u8 prev;
+
                verbose("%s: device status -> %#x\n", d->name, val);
                /*
                 * 4.1.4.3.1:
@@ -1778,9 +1924,86 @@ static void emulate_mmio_write(struct device *d, u32 off, u32 val, u32 mask)
                 *  The device MUST reset when 0 is written to device_status,
                 *  and present a 0 in device_status once that is done.
                 */
-               if (val == 0)
+               if (val == 0) {
                        reset_device(d);
+                       goto write_through8;
+               }
+
+               /* 2.1.1: The driver MUST NOT clear a device status bit. */
+               if (d->mmio->cfg.device_status & ~val)
+                       bad_driver(d, "unset of device status bit %#x -> %#x",
+                                  d->mmio->cfg.device_status, val);
+
+               /*
+                * 2.1.2:
+                *
+                *  The device MUST NOT consume buffers or notify the driver
+                *  before DRIVER_OK.
+                */
+               if (val & VIRTIO_CONFIG_S_DRIVER_OK
+                   && !(d->mmio->cfg.device_status & VIRTIO_CONFIG_S_DRIVER_OK))
+                       start_virtqueues(d);
+
+               /*
+                * 3.1.1:
+                *
+                *   The driver MUST follow this sequence to initialize a device:
+                *   - Reset the device.
+                *   - Set the ACKNOWLEDGE status bit: the guest OS has
+                 *     notice the device.
+                *   - Set the DRIVER status bit: the guest OS knows how
+                 *     to drive the device.
+                *   - Read device feature bits, and write the subset
+                *     of feature bits understood by the OS and driver
+                *     to the device. During this step the driver MAY
+                *     read (but MUST NOT write) the device-specific
+                *     configuration fields to check that it can
+                *     support the device before accepting it.
+                *   - Set the FEATURES_OK status bit.  The driver
+                *     MUST not accept new feature bits after this
+                *     step.
+                *   - Re-read device status to ensure the FEATURES_OK
+                *     bit is still set: otherwise, the device does
+                *     not support our subset of features and the
+                *     device is unusable.
+                *   - Perform device-specific setup, including
+                *     discovery of virtqueues for the device,
+                *     optional per-bus setup, reading and possibly
+                *     writing the device’s virtio configuration
+                *     space, and population of virtqueues.
+                *   - Set the DRIVER_OK status bit. At this point the
+                 *     device is “live”.
+                */
+               prev = 0;
+               switch (val & ~d->mmio->cfg.device_status) {
+               case VIRTIO_CONFIG_S_DRIVER_OK:
+                       prev |= VIRTIO_CONFIG_S_FEATURES_OK; /* fall thru */
+               case VIRTIO_CONFIG_S_FEATURES_OK:
+                       prev |= VIRTIO_CONFIG_S_DRIVER; /* fall thru */
+               case VIRTIO_CONFIG_S_DRIVER:
+                       prev |= VIRTIO_CONFIG_S_ACKNOWLEDGE; /* fall thru */
+               case VIRTIO_CONFIG_S_ACKNOWLEDGE:
+                       break;
+               default:
+                       bad_driver(d, "unknown device status bit %#x -> %#x",
+                                  d->mmio->cfg.device_status, val);
+               }
+               if (d->mmio->cfg.device_status != prev)
+                       bad_driver(d, "unexpected status transition %#x -> %#x",
+                                  d->mmio->cfg.device_status, val);
+
+               /* If they just wrote FEATURES_OK, we make sure they read */
+               switch (val & ~d->mmio->cfg.device_status) {
+               case VIRTIO_CONFIG_S_FEATURES_OK:
+                       d->wrote_features_ok = true;
+                       break;
+               case VIRTIO_CONFIG_S_DRIVER_OK:
+                       if (d->wrote_features_ok)
+                               bad_driver(d, "did not re-read FEATURES_OK");
+                       break;
+               }
                goto write_through8;
+       }
        case offsetof(struct virtio_pci_mmio, cfg.queue_select):
                vq = vq_by_num(d, val);
                /*
@@ -1808,35 +2031,45 @@ static void emulate_mmio_write(struct device *d, u32 off, u32 val, u32 mask)
                 *  to queue_size.
                 */
                if (val & (val-1))
-                       errx(1, "%s: invalid queue size %u\n", d->name, val);
+                       bad_driver(d, "invalid queue size %u", val);
                if (d->mmio->cfg.queue_enable)
-                       errx(1, "%s: changing queue size on live device",
-                            d->name);
+                       bad_driver(d, "changing queue size on live device");
                goto write_through16;
        case offsetof(struct virtio_pci_mmio, cfg.queue_msix_vector):
-               errx(1, "%s: attempt to set MSIX vector to %u",
-                    d->name, val);
-       case offsetof(struct virtio_pci_mmio, cfg.queue_enable):
+               bad_driver(d, "attempt to set MSIX vector to %u", val);
+       case offsetof(struct virtio_pci_mmio, cfg.queue_enable): {
+               struct virtqueue *vq = vq_by_num(d, d->mmio->cfg.queue_select);
+
                /*
                 * 4.1.4.3.2:
                 *
                 *  The driver MUST NOT write a 0 to queue_enable.
                 */
                if (val != 1)
-                       errx(1, "%s: setting queue_enable to %u", d->name, val);
-               d->mmio->cfg.queue_enable = val;
-               save_vq_config(&d->mmio->cfg,
-                              vq_by_num(d, d->mmio->cfg.queue_select));
+                       bad_driver(d, "setting queue_enable to %u", val);
+
                /*
-                * 4.1.4.3.2:
+                * 3.1.1:
                 *
-                *  The driver MUST configure the other virtqueue fields before
-                *  enabling the virtqueue with queue_enable.
+                *  7. Perform device-specific setup, including discovery of
+                *     virtqueues for the device, optional per-bus setup,
+                *     reading and possibly writing the device’s virtio
+                *     configuration space, and population of virtqueues.
+                *  8. Set the DRIVER_OK status bit.
+                *
+                * All our devices require all virtqueues to be enabled, so
+                * they should have done that before setting DRIVER_OK.
                 */
-               enable_virtqueue(d, vq_by_num(d, d->mmio->cfg.queue_select));
+               if (d->mmio->cfg.device_status & VIRTIO_CONFIG_S_DRIVER_OK)
+                       bad_driver(d, "enabling vq after DRIVER_OK");
+
+               d->mmio->cfg.queue_enable = val;
+               save_vq_config(&d->mmio->cfg, vq);
+               check_virtqueue(d, vq);
                goto write_through16;
+       }
        case offsetof(struct virtio_pci_mmio, cfg.queue_notify_off):
-               errx(1, "%s: attempt to write to queue_notify_off", d->name);
+               bad_driver(d, "attempt to write to queue_notify_off");
        case offsetof(struct virtio_pci_mmio, cfg.queue_desc_lo):
        case offsetof(struct virtio_pci_mmio, cfg.queue_desc_hi):
        case offsetof(struct virtio_pci_mmio, cfg.queue_avail_lo):
@@ -1850,18 +2083,36 @@ static void emulate_mmio_write(struct device *d, u32 off, u32 val, u32 mask)
                 *  enabling the virtqueue with queue_enable.
                 */
                if (d->mmio->cfg.queue_enable)
-                       errx(1, "%s: changing queue on live device",
-                            d->name);
+                       bad_driver(d, "changing queue on live device");
+
+               /*
+                * 3.1.1:
+                *
+                *  The driver MUST follow this sequence to initialize a device:
+                *...
+                *  5. Set the FEATURES_OK status bit. The driver MUST not
+                *  accept new feature bits after this step.
+                */
+               if (!(d->mmio->cfg.device_status & VIRTIO_CONFIG_S_FEATURES_OK))
+                       bad_driver(d, "setting up vq before FEATURES_OK");
+
+               /*
+                *  6. Re-read device status to ensure the FEATURES_OK bit is
+                *     still set...
+                */
+               if (d->wrote_features_ok)
+                       bad_driver(d, "didn't re-read FEATURES_OK before setup");
+
                goto write_through32;
        case offsetof(struct virtio_pci_mmio, notify):
                vq = vq_by_num(d, val);
                if (!vq)
-                       errx(1, "Invalid vq notification on %u", val);
+                       bad_driver(d, "Invalid vq notification on %u", val);
                /* Notify the process handling this vq by adding 1 to eventfd */
                write(vq->eventfd, "\1\0\0\0\0\0\0\0", 8);
                goto write_through16;
        case offsetof(struct virtio_pci_mmio, isr):
-               errx(1, "%s: Unexpected write to isr", d->name);
+               bad_driver(d, "Unexpected write to isr");
        /* Weird corner case: write to emerg_wr of console */
        case sizeof(struct virtio_pci_mmio)
                + offsetof(struct virtio_console_config, emerg_wr):
@@ -1878,9 +2129,28 @@ static void emulate_mmio_write(struct device *d, u32 off, u32 val, u32 mask)
                 *   The driver MUST NOT write to device_feature, num_queues,
                 *   config_generation or queue_notify_off.
                 */
-               errx(1, "%s: Unexpected write to offset %u", d->name, off);
+               bad_driver(d, "Unexpected write to offset %u", off);
        }
 
+feature_write_through32:
+       /*
+        * 3.1.1:
+        *
+        *   The driver MUST follow this sequence to initialize a device:
+        *...
+        *   - Set the DRIVER status bit: the guest OS knows how
+        *     to drive the device.
+        *   - Read device feature bits, and write the subset
+        *     of feature bits understood by the OS and driver
+        *     to the device.
+        *...
+        *   - Set the FEATURES_OK status bit. The driver MUST not
+        *     accept new feature bits after this step.
+        */
+       if (!(d->mmio->cfg.device_status & VIRTIO_CONFIG_S_DRIVER))
+               bad_driver(d, "feature write before VIRTIO_CONFIG_S_DRIVER");
+       if (d->mmio->cfg.device_status & VIRTIO_CONFIG_S_FEATURES_OK)
+               bad_driver(d, "feature write after VIRTIO_CONFIG_S_FEATURES_OK");
 
        /*
         * 4.1.3.1:
@@ -1891,8 +2161,8 @@ static void emulate_mmio_write(struct device *d, u32 off, u32 val, u32 mask)
         */
 write_through32:
        if (mask != 0xFFFFFFFF) {
-               errx(1, "%s: non-32-bit write to offset %u (%#x)",
-                    d->name, off, getreg(eip));
+               bad_driver(d, "non-32-bit write to offset %u (%#x)",
+                          off, getreg(eip));
                return;
        }
        memcpy((char *)d->mmio + off, &val, 4);
@@ -1900,15 +2170,15 @@ write_through32:
 
 write_through16:
        if (mask != 0xFFFF)
-               errx(1, "%s: non-16-bit (%#x) write to offset %u (%#x)",
-                    d->name, mask, off, getreg(eip));
+               bad_driver(d, "non-16-bit write to offset %u (%#x)",
+                          off, getreg(eip));
        memcpy((char *)d->mmio + off, &val, 2);
        return;
 
 write_through8:
        if (mask != 0xFF)
-               errx(1, "%s: non-8-bit write to offset %u (%#x)",
-                    d->name, off, getreg(eip));
+               bad_driver(d, "non-8-bit write to offset %u (%#x)",
+                          off, getreg(eip));
        memcpy((char *)d->mmio + off, &val, 1);
        return;
 }
@@ -1923,12 +2193,29 @@ static u32 emulate_mmio_read(struct device *d, u32 off, u32 mask)
        case offsetof(struct virtio_pci_mmio, cfg.device_feature):
        case offsetof(struct virtio_pci_mmio, cfg.guest_feature_select):
        case offsetof(struct virtio_pci_mmio, cfg.guest_feature):
+               /*
+                * 3.1.1:
+                *
+                *   The driver MUST follow this sequence to initialize a device:
+                *...
+                *   - Set the DRIVER status bit: the guest OS knows how
+                *     to drive the device.
+                *   - Read device feature bits, and write the subset
+                *     of feature bits understood by the OS and driver
+                *     to the device.
+                */
+               if (!(d->mmio->cfg.device_status & VIRTIO_CONFIG_S_DRIVER))
+                       bad_driver(d,
+                                  "feature read before VIRTIO_CONFIG_S_DRIVER");
                goto read_through32;
        case offsetof(struct virtio_pci_mmio, cfg.msix_config):
-               errx(1, "%s: read of msix_config", d->name);
+               bad_driver(d, "read of msix_config");
        case offsetof(struct virtio_pci_mmio, cfg.num_queues):
                goto read_through16;
        case offsetof(struct virtio_pci_mmio, cfg.device_status):
+               /* As they did read, any write of FEATURES_OK is now fine. */
+               d->wrote_features_ok = false;
+               goto read_through8;
        case offsetof(struct virtio_pci_mmio, cfg.config_generation):
                /*
                 * 4.1.4.3.1:
@@ -1943,11 +2230,19 @@ static u32 emulate_mmio_read(struct device *d, u32 off, u32 mask)
                 */
                goto read_through8;
        case offsetof(struct virtio_pci_mmio, notify):
+               /*
+                * 3.1.1:
+                *
+                *   The driver MUST NOT notify the device before setting
+                *   DRIVER_OK.
+                */
+               if (!(d->mmio->cfg.device_status & VIRTIO_CONFIG_S_DRIVER_OK))
+                       bad_driver(d, "notify before VIRTIO_CONFIG_S_DRIVER_OK");
                goto read_through16;
        case offsetof(struct virtio_pci_mmio, isr):
                if (mask != 0xFF)
-                       errx(1, "%s: non-8-bit read from offset %u (%#x)",
-                            d->name, off, getreg(eip));
+                       bad_driver(d, "non-8-bit read from offset %u (%#x)",
+                                  off, getreg(eip));
                isr = d->mmio->isr;
                /*
                 * 4.1.4.5.1:
@@ -1957,13 +2252,28 @@ static u32 emulate_mmio_read(struct device *d, u32 off, u32 mask)
                d->mmio->isr = 0;
                return isr;
        case offsetof(struct virtio_pci_mmio, padding):
-               errx(1, "%s: read from padding (%#x)",
-                    d->name, getreg(eip));
+               bad_driver(d, "read from padding (%#x)", getreg(eip));
        default:
                /* Read from device config space, beware unaligned overflow */
                if (off > d->mmio_size - 4)
-                       errx(1, "%s: read past end (%#x)",
-                            d->name, getreg(eip));
+                       bad_driver(d, "read past end (%#x)", getreg(eip));
+
+               /*
+                * 3.1.1:
+                *  The driver MUST follow this sequence to initialize a device:
+                *...
+                *  3. Set the DRIVER status bit: the guest OS knows how to
+                *  drive the device.
+                *  4. Read device feature bits, and write the subset of
+                *  feature bits understood by the OS and driver to the
+                *  device. During this step the driver MAY read (but MUST NOT
+                *  write) the device-specific configuration fields to check
+                *  that it can support the device before accepting it.
+                */
+               if (!(d->mmio->cfg.device_status & VIRTIO_CONFIG_S_DRIVER))
+                       bad_driver(d,
+                                  "config read before VIRTIO_CONFIG_S_DRIVER");
+
                if (mask == 0xFFFFFFFF)
                        goto read_through32;
                else if (mask == 0xFFFF)
@@ -1981,22 +2291,22 @@ static u32 emulate_mmio_read(struct device *d, u32 off, u32 mask)
         */
 read_through32:
        if (mask != 0xFFFFFFFF)
-               errx(1, "%s: non-32-bit read to offset %u (%#x)",
-                    d->name, off, getreg(eip));
+               bad_driver(d, "non-32-bit read to offset %u (%#x)",
+                          off, getreg(eip));
        memcpy(&val, (char *)d->mmio + off, 4);
        return val;
 
 read_through16:
        if (mask != 0xFFFF)
-               errx(1, "%s: non-16-bit read to offset %u (%#x)",
-                    d->name, off, getreg(eip));
+               bad_driver(d, "non-16-bit read to offset %u (%#x)",
+                          off, getreg(eip));
        memcpy(&val, (char *)d->mmio + off, 2);
        return val;
 
 read_through8:
        if (mask != 0xFF)
-               errx(1, "%s: non-8-bit read to offset %u (%#x)",
-                    d->name, off, getreg(eip));
+               bad_driver(d, "non-8-bit read to offset %u (%#x)",
+                          off, getreg(eip));
        memcpy(&val, (char *)d->mmio + off, 1);
        return val;
 }
@@ -2064,7 +2374,8 @@ static void emulate_mmio(unsigned long paddr, const u8 *insn)
  * routines to allocate and manage them.
  */
 static void add_pci_virtqueue(struct device *dev,
-                             void (*service)(struct virtqueue *))
+                             void (*service)(struct virtqueue *),
+                             const char *name)
 {
        struct virtqueue **i, *vq = malloc(sizeof(*vq));
 
@@ -2072,6 +2383,7 @@ static void add_pci_virtqueue(struct device *dev,
        vq->next = NULL;
        vq->last_avail_idx = 0;
        vq->dev = dev;
+       vq->name = name;
 
        /*
         * This is the routine the service thread will run, and its Process ID
@@ -2172,6 +2484,10 @@ static void init_pci_config(struct pci_config *pci, u16 type,
         *
         *  The device MUST either present notify_off_multiplier as an even
         *  power of 2, or present notify_off_multiplier as 0.
+        *
+        * 2.1.2:
+        *
+        *   The device MUST initialize device status to 0 upon reset. 
         */
        memset(pci, 0, sizeof(*pci));
 
@@ -2312,6 +2628,7 @@ static struct device *new_pci_device(const char *name, u16 type,
        dev->name = name;
        dev->vq = NULL;
        dev->running = false;
+       dev->wrote_features_ok = false;
        dev->mmio_size = sizeof(struct virtio_pci_mmio);
        dev->mmio = calloc(1, dev->mmio_size);
        dev->features = (u64)1 << VIRTIO_F_VERSION_1;
@@ -2359,8 +2676,8 @@ static void setup_console(void)
         * stdin.  When they put something in the output queue, we write it to
         * stdout.
         */
-       add_pci_virtqueue(dev, console_input);
-       add_pci_virtqueue(dev, console_output);
+       add_pci_virtqueue(dev, console_input, "input");
+       add_pci_virtqueue(dev, console_output, "output");
 
        /* We need a configuration area for the emerg_wr early writes. */
        add_pci_feature(dev, VIRTIO_CONSOLE_F_EMERG_WRITE);
@@ -2500,7 +2817,7 @@ static int get_tun_device(char tapif[IFNAMSIZ])
         * about our expanded header (which is called
         * virtio_net_hdr_mrg_rxbuf in the legacy system).
         */
-       vnet_hdr_sz = sizeof(struct virtio_net_hdr_mrg_rxbuf);
+       vnet_hdr_sz = sizeof(struct virtio_net_hdr_v1);
        if (ioctl(netfd, TUNSETVNETHDRSZ, &vnet_hdr_sz) != 0)
                err(1, "Setting tun header size to %u", vnet_hdr_sz);
 
@@ -2531,8 +2848,8 @@ static void setup_tun_net(char *arg)
        dev->priv = net_info;
 
        /* Network devices need a recv and a send queue, just like console. */
-       add_pci_virtqueue(dev, net_input);
-       add_pci_virtqueue(dev, net_output);
+       add_pci_virtqueue(dev, net_input, "rx");
+       add_pci_virtqueue(dev, net_output, "tx");
 
        /*
         * We need a socket to perform the magic network ioctls to bring up the
@@ -2631,7 +2948,7 @@ static void blk_request(struct virtqueue *vq)
        head = wait_for_vq_desc(vq, iov, &out_num, &in_num);
 
        /* Copy the output header from the front of the iov (adjusts iov) */
-       iov_consume(iov, out_num, &out, sizeof(out));
+       iov_consume(vq->dev, iov, out_num, &out, sizeof(out));
 
        /* Find and trim end of iov input array, for our status byte. */
        in = NULL;
@@ -2643,7 +2960,7 @@ static void blk_request(struct virtqueue *vq)
                }
        }
        if (!in)
-               errx(1, "Bad virtblk cmd with no room for status");
+               bad_driver_vq(vq, "Bad virtblk cmd with no room for status");
 
        /*
         * For historical reasons, block operations are expressed in 512 byte
@@ -2673,7 +2990,7 @@ static void blk_request(struct virtqueue *vq)
                        /* Trim it back to the correct length */
                        ftruncate64(vblk->fd, vblk->len);
                        /* Die, bad Guest, die. */
-                       errx(1, "Write past end %llu+%u", off, ret);
+                       bad_driver_vq(vq, "Write past end %llu+%u", off, ret);
                }
 
                wlen = sizeof(*in);
@@ -2719,7 +3036,7 @@ static void setup_block_file(const char *filename)
        dev = new_pci_device("block", VIRTIO_ID_BLOCK, 0x01, 0x80);
 
        /* The device has one virtqueue, where the Guest places requests. */
-       add_pci_virtqueue(dev, blk_request);
+       add_pci_virtqueue(dev, blk_request, "request");
 
        /* Allocate the room for our own bookkeeping */
        vblk = dev->priv = malloc(sizeof(*vblk));
@@ -2766,7 +3083,7 @@ static void rng_input(struct virtqueue *vq)
        /* First we need a buffer from the Guests's virtqueue. */
        head = wait_for_vq_desc(vq, iov, &out_num, &in_num);
        if (out_num)
-               errx(1, "Output buffers in rng?");
+               bad_driver_vq(vq, "Output buffers in rng?");
 
        /*
         * Just like the console write, we loop to cover the whole iovec.
@@ -2776,7 +3093,7 @@ static void rng_input(struct virtqueue *vq)
                len = readv(rng_info->rfd, iov, in_num);
                if (len <= 0)
                        err(1, "Read from /dev/urandom gave %i", len);
-               iov_consume(iov, in_num, NULL, len);
+               iov_consume(vq->dev, iov, in_num, NULL, len);
                totlen += len;
        }
 
@@ -2800,7 +3117,7 @@ static void setup_rng(void)
        dev->priv = rng_info;
 
        /* The device has one virtqueue, where the Guest places inbufs. */
-       add_pci_virtqueue(dev, rng_input);
+       add_pci_virtqueue(dev, rng_input, "input");
 
        /* We don't have any configuration space */
        no_device_config(dev);