3.14-stable review patch.  If anyone has any objections, please let me know.

------------------

From: "Michael S. Tsirkin" <[email protected]>

commit 6fbc198cf623944ab60a1db6d306a4d55cdd820d upstream.

On restore, virtio pci does the following:
+ set features
+ init vqs etc - device can be used at this point!
+ set ACKNOWLEDGE,DRIVER and DRIVER_OK status bits

This is in violation of the virtio spec, which
requires the following order:
- ACKNOWLEDGE
- DRIVER
- init vqs
- DRIVER_OK

This behaviour will break with hypervisors that assume spec compliant
behaviour.  It seems like a good idea to have this patch applied to
stable branches to reduce the support butden for the hypervisors.

Cc: Amit Shah <[email protected]>
Signed-off-by: Michael S. Tsirkin <[email protected]>
Signed-off-by: Rusty Russell <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>

---
 drivers/virtio/virtio_pci.c |   33 ++++++++++++++++++++++++++++++---
 1 file changed, 30 insertions(+), 3 deletions(-)

--- a/drivers/virtio/virtio_pci.c
+++ b/drivers/virtio/virtio_pci.c
@@ -791,6 +791,7 @@ static int virtio_pci_restore(struct dev
        struct pci_dev *pci_dev = to_pci_dev(dev);
        struct virtio_pci_device *vp_dev = pci_get_drvdata(pci_dev);
        struct virtio_driver *drv;
+       unsigned status = 0;
        int ret;
 
        drv = container_of(vp_dev->vdev.dev.driver,
@@ -801,14 +802,40 @@ static int virtio_pci_restore(struct dev
                return ret;
 
        pci_set_master(pci_dev);
+       /* We always start by resetting the device, in case a previous
+        * driver messed it up. */
+       vp_reset(&vp_dev->vdev);
+
+       /* Acknowledge that we've seen the device. */
+       status |= VIRTIO_CONFIG_S_ACKNOWLEDGE;
+       vp_set_status(&vp_dev->vdev, status);
+
+       /* Maybe driver failed before freeze.
+        * Restore the failed status, for debugging. */
+       status |= vp_dev->saved_status & VIRTIO_CONFIG_S_FAILED;
+       vp_set_status(&vp_dev->vdev, status);
+
+       if (!drv)
+               return 0;
+
+       /* We have a driver! */
+       status |= VIRTIO_CONFIG_S_DRIVER;
+       vp_set_status(&vp_dev->vdev, status);
+
        vp_finalize_features(&vp_dev->vdev);
 
-       if (drv && drv->restore)
+       if (drv->restore) {
                ret = drv->restore(&vp_dev->vdev);
+               if (ret) {
+                       status |= VIRTIO_CONFIG_S_FAILED;
+                       vp_set_status(&vp_dev->vdev, status);
+                       return ret;
+               }
+       }
 
        /* Finally, tell the device we're all set */
-       if (!ret)
-               vp_set_status(&vp_dev->vdev, vp_dev->saved_status);
+       status |= VIRTIO_CONFIG_S_DRIVER_OK;
+       vp_set_status(&vp_dev->vdev, status);
 
        return ret;
 }


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Reply via email to