lguest: Simplify device initialization.

We used to notify the Host every time we updated a device's status.  However,
it only really needs to know when we're resetting the device, or failed to
initialize it, or when we've finished our feature negotiation.

In particular, we used to wait for VIRTIO_CONFIG_S_DRIVER_OK in the
status byte before starting the device service threads.  But this
corresponds to the successful finish of device initialization, which
might (like virtio_blk's partition scanning) use the device.  So we
had a hack, if they used the device before we expected we started the
threads anyway.

Now we hook into the finalize_features hook in the Guest: at that
point we tell the Launcher that it can rely on the features we have
acked.  On the Launcher side, we look at the status at that point, and
start servicing the device.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

diff --git a/Documentation/virtual/lguest/lguest.c b/Documentation/virtual/lguest/lguest.c
index e3b9bb7a644a90a359622dcdfe757bce259ee586..80261d34da3de8dfbbefb3e5169e8e2ac633b128 100644 (file)
--- a/Documentation/virtual/lguest/lguest.c
+++ b/Documentation/virtual/lguest/lguest.c
@@ -1095,9 +1095,10 @@ static void update_device_status(struct device *dev)
                warnx("Device %s configuration FAILED", dev->name);
                if (dev->running)
                        reset_device(dev);
-       } else if (dev->desc->status & VIRTIO_CONFIG_S_DRIVER_OK) {
-               if (!dev->running)
-                       start_device(dev);
+       } else {
+               if (dev->running)
+                       err(1, "Device %s features finalized twice", dev->name);
+               start_device(dev);
        }
 }
 
@@ -1122,25 +1123,11 @@ static void handle_output(unsigned long addr)
                        return;
                }
 
-               /*
-                * Devices *can* be used before status is set to DRIVER_OK.
-                * The original plan was that they would never do this: they
-                * would always finish setting up their status bits before
-                * actually touching the virtqueues.  In practice, we allowed
-                * them to, and they do (eg. the disk probes for partition
-                * tables as part of initialization).
-                *
-                * If we see this, we start the device: once it's running, we
-                * expect the device to catch all the notifications.
-                */
+               /* Devices should not be used before features are finalized. */
                for (vq = i->vq; vq; vq = vq->next) {
                        if (addr != vq->config.pfn*getpagesize())
                                continue;
-                       if (i->running)
-                               errx(1, "Notification on running %s", i->name);
-                       /* This just calls create_thread() for each virtqueue */
-                       start_device(i);
-                       return;
+                       errx(1, "Notification on %s before setup!", i->name);
                }
        }
 

diff --git a/drivers/lguest/lguest_device.c b/drivers/lguest/lguest_device.c
index 69c84a1d88ea982d164ca55a94a9c8544c0cceb9..5289ffa2e500c77ac3a2c0c1c5cddc1332a07b7b 100644 (file)
--- a/drivers/lguest/lguest_device.c
+++ b/drivers/lguest/lguest_device.c
@@ -108,6 +108,17 @@ static u32 lg_get_features(struct virtio_device *vdev)
        return features;
 }
 
+/*
+ * To notify on reset or feature finalization, we (ab)use the NOTIFY
+ * hypercall, with the descriptor address of the device.
+ */
+static void status_notify(struct virtio_device *vdev)
+{
+       unsigned long offset = (void *)to_lgdev(vdev)->desc - lguest_devices;
+
+       hcall(LHCALL_NOTIFY, (max_pfn << PAGE_SHIFT) + offset, 0, 0, 0);
+}
+
 /*
  * The virtio core takes the features the Host offers, and copies the ones
  * supported by the driver into the vdev->features array.  Once that's all
@@ -135,6 +146,9 @@ static void lg_finalize_features(struct virtio_device *vdev)
                if (test_bit(i, vdev->features))
                        out_features[i / 8] |= (1 << (i % 8));
        }
+
+       /* Tell Host we've finished with this device's feature negotiation */
+       status_notify(vdev);
 }
 
 /* Once they've found a field, getting a copy of it is easy. */
@@ -168,28 +182,21 @@ static u8 lg_get_status(struct virtio_device *vdev)
        return to_lgdev(vdev)->desc->status;
 }
 
-/*
- * To notify on status updates, we (ab)use the NOTIFY hypercall, with the
- * descriptor address of the device.  A zero status means "reset".
- */
-static void set_status(struct virtio_device *vdev, u8 status)
-{
-       unsigned long offset = (void *)to_lgdev(vdev)->desc - lguest_devices;
-
-       /* We set the status. */
-       to_lgdev(vdev)->desc->status = status;
-       hcall(LHCALL_NOTIFY, (max_pfn << PAGE_SHIFT) + offset, 0, 0, 0);
-}
-
 static void lg_set_status(struct virtio_device *vdev, u8 status)
 {
        BUG_ON(!status);
-       set_status(vdev, status);
+       to_lgdev(vdev)->desc->status = status;
+
+       /* Tell Host immediately if we failed. */
+       if (status & VIRTIO_CONFIG_S_FAILED)
+               status_notify(vdev);
 }
 
 static void lg_reset(struct virtio_device *vdev)
 {
-       set_status(vdev, 0);
+       /* 0 status means "reset" */
+       to_lgdev(vdev)->desc->status = 0;
+       status_notify(vdev);
 }
 
 /*