| From 6fbc198cf623944ab60a1db6d306a4d55cdd820d Mon Sep 17 00:00:00 2001 |
| From: "Michael S. Tsirkin" <mst@redhat.com> |
| Date: Tue, 14 Oct 2014 10:40:29 +1030 |
| Subject: virtio_pci: fix virtio spec compliance on restore |
| |
| From: "Michael S. Tsirkin" <mst@redhat.com> |
| |
| 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 <amit.shah@redhat.com> |
| Signed-off-by: Michael S. Tsirkin <mst@redhat.com> |
| Signed-off-by: Rusty Russell <rusty@rustcorp.com.au> |
| Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> |
| |
| --- |
| 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 |
| @@ -789,6 +789,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, |
| @@ -799,14 +800,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; |
| } |