All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Michael S. Tsirkin" <mst@redhat.com>
To: linux-kernel@vger.kernel.org
Cc: linux-s390@vger.kernel.org, kvm@vger.kernel.org,
	linux-scsi@vger.kernel.org,
	Christian Borntraeger <borntraeger@de.ibm.com>,
	netdev@vger.kernel.org, stable@vger.kernel.org,
	virtualization@lists.linux-foundation.org,
	Paolo Bonzini <pbonzini@redhat.com>,
	Amit Shah <amit.shah@redhat.com>,
	v9fs-developer@lists.sourceforge.net,
	"David S. Miller" <davem@davemloft.net>
Subject: [PATCH v3 01/25] virtio_pci: fix virtio spec compliance on restore
Date: Sun, 12 Oct 2014 14:46:28 +0300	[thread overview]
Message-ID: <1413114332-626-2-git-send-email-mst@redhat.com> (raw)
In-Reply-To: <1413114332-626-1-git-send-email-mst@redhat.com>

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: stable@vger.kernel.org
Cc: Amit Shah <amit.shah@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 drivers/virtio/virtio_pci.c | 33 ++++++++++++++++++++++++++++++---
 1 file changed, 30 insertions(+), 3 deletions(-)

diff --git a/drivers/virtio/virtio_pci.c b/drivers/virtio/virtio_pci.c
index 3d1463c..add40d0 100644
--- a/drivers/virtio/virtio_pci.c
+++ b/drivers/virtio/virtio_pci.c
@@ -789,6 +789,7 @@ static int virtio_pci_restore(struct device *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 device *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;
 }
-- 
MST

WARNING: multiple messages have this Message-ID (diff)
From: "Michael S. Tsirkin" <mst@redhat.com>
To: linux-kernel@vger.kernel.org
Cc: Rusty Russell <rusty@rustcorp.com.au>,
	virtualization@lists.linux-foundation.org,
	linux-scsi@vger.kernel.org, linux-s390@vger.kernel.org,
	v9fs-developer@lists.sourceforge.net, netdev@vger.kernel.org,
	kvm@vger.kernel.org, Amit Shah <amit.shah@redhat.com>,
	Cornelia Huck <cornelia.huck@de.ibm.com>,
	Christian Borntraeger <borntraeger@de.ibm.com>,
	"David S. Miller" <davem@davemloft.net>,
	Paolo Bonzini <pbonzini@redhat.com>,
	stable@vger.kernel.org
Subject: [PATCH v3 01/25] virtio_pci: fix virtio spec compliance on restore
Date: Sun, 12 Oct 2014 14:46:28 +0300	[thread overview]
Message-ID: <1413114332-626-2-git-send-email-mst@redhat.com> (raw)
In-Reply-To: <1413114332-626-1-git-send-email-mst@redhat.com>

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: stable@vger.kernel.org
Cc: Amit Shah <amit.shah@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 drivers/virtio/virtio_pci.c | 33 ++++++++++++++++++++++++++++++---
 1 file changed, 30 insertions(+), 3 deletions(-)

diff --git a/drivers/virtio/virtio_pci.c b/drivers/virtio/virtio_pci.c
index 3d1463c..add40d0 100644
--- a/drivers/virtio/virtio_pci.c
+++ b/drivers/virtio/virtio_pci.c
@@ -789,6 +789,7 @@ static int virtio_pci_restore(struct device *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 device *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;
 }
-- 
MST


  reply	other threads:[~2014-10-12 11:46 UTC|newest]

Thread overview: 56+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-10-12 11:46 [PATCH v3 00/25] virtio: fix spec compliance issues Michael S. Tsirkin
2014-10-12 11:46 ` Michael S. Tsirkin
2014-10-12 11:46 ` Michael S. Tsirkin [this message]
2014-10-12 11:46   ` [PATCH v3 01/25] virtio_pci: fix virtio spec compliance on restore Michael S. Tsirkin
2014-10-12 11:46 ` [PATCH v3 02/25] virtio: unify config_changed handling Michael S. Tsirkin
2014-10-12 11:46   ` Michael S. Tsirkin
2014-10-12 11:46 ` [PATCH v3 03/25] virtio-pci: move freeze/restore to virtio core Michael S. Tsirkin
2014-10-12 11:46   ` Michael S. Tsirkin
2014-10-12 11:46 ` [PATCH v3 04/25] virtio: defer config changed notifications Michael S. Tsirkin
2014-10-12 11:46   ` Michael S. Tsirkin
2014-10-12 11:46 ` [PATCH v3 05/25] virtio_blk: drop config_enable Michael S. Tsirkin
2014-10-12 11:46   ` Michael S. Tsirkin
2014-10-12 11:46 ` [PATCH v3 06/25] virtio-blk: drop config_mutex Michael S. Tsirkin
2014-10-12 11:46   ` Michael S. Tsirkin
2014-10-12 11:47 ` [PATCH v3 07/25] virtio_net: drop config_enable Michael S. Tsirkin
2014-10-12 11:47 ` Michael S. Tsirkin
2014-10-12 11:47 ` [PATCH v3 08/25] virtio-net: drop config_mutex Michael S. Tsirkin
2014-10-12 11:47   ` Michael S. Tsirkin
2014-10-12 11:47 ` [PATCH v3 09/25] virtio_net: minor cleanup Michael S. Tsirkin
2014-10-12 11:47 ` Michael S. Tsirkin
2014-10-12 11:47 ` [PATCH v3 10/25] virtio: add API to enable VQs early Michael S. Tsirkin
2014-10-12 11:47   ` Michael S. Tsirkin
2014-10-13  6:52   ` Rusty Russell
2014-10-13  6:52     ` Rusty Russell
2014-10-13  7:35     ` Michael S. Tsirkin
2014-10-13  7:35       ` Michael S. Tsirkin
2014-10-12 11:47 ` [PATCH v3 11/25] virtio_net: " Michael S. Tsirkin
2014-10-12 11:47 ` Michael S. Tsirkin
2014-10-12 11:47 ` [PATCH v3 12/25] virtio_blk: " Michael S. Tsirkin
2014-10-12 11:47 ` Michael S. Tsirkin
2014-10-12 11:47 ` [PATCH v3 13/25] virtio_console: " Michael S. Tsirkin
2014-10-12 11:47 ` Michael S. Tsirkin
2014-10-12 11:47 ` [PATCH v3 14/25] 9p/trans_virtio: " Michael S. Tsirkin
2014-10-12 11:47 ` Michael S. Tsirkin
2014-10-12 11:47 ` [PATCH v3 15/25] virtio_net: fix use after free on allocation failure Michael S. Tsirkin
2014-10-12 11:47   ` Michael S. Tsirkin
2014-10-12 11:47 ` [PATCH v3 16/25] virtio_scsi: move kick event out from virtscsi_init Michael S. Tsirkin
2014-10-12 11:47 ` Michael S. Tsirkin
2014-10-12 11:47 ` [PATCH v3 17/25] virtio_blk: enable VQs early on restore Michael S. Tsirkin
2014-10-12 11:47 ` Michael S. Tsirkin
2014-10-12 11:48 ` [PATCH v3 18/25] virtio_scsi: " Michael S. Tsirkin
2014-10-12 11:48   ` Michael S. Tsirkin
2014-10-12 11:48 ` [PATCH v3 19/25] virtio_console: " Michael S. Tsirkin
2014-10-12 11:48   ` Michael S. Tsirkin
2014-10-12 11:48 ` [PATCH v3 20/25] virtio_net: " Michael S. Tsirkin
2014-10-12 11:48 ` Michael S. Tsirkin
2014-10-12 11:48 ` [PATCH v3 21/25] virito_scsi: use freezable WQ for events Michael S. Tsirkin
2014-10-12 11:48 ` Michael S. Tsirkin
2014-10-12 11:48 ` [PATCH v3 22/25] virtio_scsi: fix race on device removal Michael S. Tsirkin
2014-10-12 11:48 ` Michael S. Tsirkin
2014-10-12 11:48 ` [PATCH v3 23/25] virtio_balloon: enable VQs early on restore Michael S. Tsirkin
2014-10-12 11:48 ` Michael S. Tsirkin
2014-10-12 11:48 ` [PATCH v3 24/25] virtio_scsi: drop scan callback Michael S. Tsirkin
2014-10-12 11:48 ` Michael S. Tsirkin
2014-10-12 11:48 ` [PATCH v3 25/25] virtio-rng: refactor probe error handling Michael S. Tsirkin
2014-10-12 11:48 ` Michael S. Tsirkin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1413114332-626-2-git-send-email-mst@redhat.com \
    --to=mst@redhat.com \
    --cc=amit.shah@redhat.com \
    --cc=borntraeger@de.ibm.com \
    --cc=davem@davemloft.net \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-s390@vger.kernel.org \
    --cc=linux-scsi@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=pbonzini@redhat.com \
    --cc=stable@vger.kernel.org \
    --cc=v9fs-developer@lists.sourceforge.net \
    --cc=virtualization@lists.linux-foundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.