9ae3a8
From 20180f303f6fd602ca8fd66bdd746916184177de Mon Sep 17 00:00:00 2001
9ae3a8
From: Paolo Bonzini <pbonzini@redhat.com>
9ae3a8
Date: Mon, 23 Sep 2013 17:08:02 +0200
9ae3a8
Subject: [PATCH 04/29] virtio-blk: do not relay a previous driver's WCE configuration to the current
9ae3a8
9ae3a8
RH-Author: Paolo Bonzini <pbonzini@redhat.com>
9ae3a8
Message-id: <1379956082-3646-3-git-send-email-pbonzini@redhat.com>
9ae3a8
Patchwork-id: 54492
9ae3a8
O-Subject: [RHEL 7.0 qemu-kvm PATCH 2/2] virtio-blk: do not relay a previous driver's WCE configuration to the current
9ae3a8
Bugzilla: 1009993
9ae3a8
RH-Acked-by: Stefan Hajnoczi <stefanha@redhat.com>
9ae3a8
RH-Acked-by: Fam Zheng <famz@redhat.com>
9ae3a8
RH-Acked-by: Kevin Wolf <kwolf@redhat.com>
9ae3a8
9ae3a8
The following sequence happens:
9ae3a8
- the SeaBIOS virtio-blk driver does not support the WCE feature, which
9ae3a8
causes QEMU to disable writeback caching
9ae3a8
9ae3a8
- the Linux virtio-blk driver resets the device, finds WCE is available
9ae3a8
but writeback caching is disabled; tells block layer to not send cache
9ae3a8
flush commands
9ae3a8
9ae3a8
- the Linux virtio-blk driver sets the DRIVER_OK bit, which causes
9ae3a8
writeback caching to be re-enabled, but the Linux virtio-blk driver does
9ae3a8
not know of this side effect and cache flushes remain disabled
9ae3a8
9ae3a8
The bug is at the third step.  If the guest does know about CONFIG_WCE,
9ae3a8
QEMU should ignore the WCE feature's state.  The guest will control the
9ae3a8
cache mode solely using configuration space.  This change makes Linux
9ae3a8
do flushes correctly, but Linux will keep SeaBIOS's writethrough mode.
9ae3a8
9ae3a8
Hence, whenever the guest is reset, the cache mode of the disk should
9ae3a8
be reset to whatever was specified in the "-drive" option.  With this
9ae3a8
change, the Linux virtio-blk driver finds that writeback caching is
9ae3a8
enabled, and tells the block layer to send cache flush commands
9ae3a8
appropriately.
9ae3a8
9ae3a8
Reported-by: Rusty Russell 
9ae3a8
Cc: qemu-stable@nongnu.org
9ae3a8
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
9ae3a8
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
9ae3a8
(cherry picked from commit ef5bc96268ceec64769617dc53b0ac3a20ff351c)
9ae3a8
---
9ae3a8
 hw/block/virtio-blk.c          | 24 ++++++++++++++++++++++--
9ae3a8
 include/hw/virtio/virtio-blk.h |  1 +
9ae3a8
 2 files changed, 23 insertions(+), 2 deletions(-)
9ae3a8
9ae3a8
Signed-off-by: Miroslav Rezanina <mrezanin@redhat.com>
9ae3a8
---
9ae3a8
 hw/block/virtio-blk.c          |   24 ++++++++++++++++++++++--
9ae3a8
 include/hw/virtio/virtio-blk.h |    1 +
9ae3a8
 2 files changed, 23 insertions(+), 2 deletions(-)
9ae3a8
9ae3a8
diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
9ae3a8
index cca0c77..ce1a523 100644
9ae3a8
--- a/hw/block/virtio-blk.c
9ae3a8
+++ b/hw/block/virtio-blk.c
9ae3a8
@@ -460,9 +460,9 @@ static void virtio_blk_dma_restart_cb(void *opaque, int running,
9ae3a8
 
9ae3a8
 static void virtio_blk_reset(VirtIODevice *vdev)
9ae3a8
 {
9ae3a8
-#ifdef CONFIG_VIRTIO_BLK_DATA_PLANE
9ae3a8
     VirtIOBlock *s = VIRTIO_BLK(vdev);
9ae3a8
 
9ae3a8
+#ifdef CONFIG_VIRTIO_BLK_DATA_PLANE
9ae3a8
     if (s->dataplane) {
9ae3a8
         virtio_blk_data_plane_stop(s->dataplane);
9ae3a8
     }
9ae3a8
@@ -473,6 +473,7 @@ static void virtio_blk_reset(VirtIODevice *vdev)
9ae3a8
      * are per-device request lists.
9ae3a8
      */
9ae3a8
     bdrv_drain_all();
9ae3a8
+    bdrv_set_enable_write_cache(s->bs, s->original_wce);
9ae3a8
 }
9ae3a8
 
9ae3a8
 /* coalesce internal state, copy to pci i/o region 0
9ae3a8
@@ -564,7 +565,25 @@ static void virtio_blk_set_status(VirtIODevice *vdev, uint8_t status)
9ae3a8
     }
9ae3a8
 
9ae3a8
     features = vdev->guest_features;
9ae3a8
-    bdrv_set_enable_write_cache(s->bs, !!(features & (1 << VIRTIO_BLK_F_WCE)));
9ae3a8
+
9ae3a8
+    /* A guest that supports VIRTIO_BLK_F_CONFIG_WCE must be able to send
9ae3a8
+     * cache flushes.  Thus, the "auto writethrough" behavior is never
9ae3a8
+     * necessary for guests that support the VIRTIO_BLK_F_CONFIG_WCE feature.
9ae3a8
+     * Leaving it enabled would break the following sequence:
9ae3a8
+     *
9ae3a8
+     *     Guest started with "-drive cache=writethrough"
9ae3a8
+     *     Guest sets status to 0
9ae3a8
+     *     Guest sets DRIVER bit in status field
9ae3a8
+     *     Guest reads host features (WCE=0, CONFIG_WCE=1)
9ae3a8
+     *     Guest writes guest features (WCE=0, CONFIG_WCE=1)
9ae3a8
+     *     Guest writes 1 to the WCE configuration field (writeback mode)
9ae3a8
+     *     Guest sets DRIVER_OK bit in status field
9ae3a8
+     *
9ae3a8
+     * s->bs would erroneously be placed in writethrough mode.
9ae3a8
+     */
9ae3a8
+    if (!(features & (1 << VIRTIO_BLK_F_CONFIG_WCE))) {
9ae3a8
+        bdrv_set_enable_write_cache(s->bs, !!(features & (1 << VIRTIO_BLK_F_WCE)));
9ae3a8
+    }
9ae3a8
 }
9ae3a8
 
9ae3a8
 static void virtio_blk_save(QEMUFile *f, void *opaque)
9ae3a8
@@ -674,6 +693,7 @@ static int virtio_blk_device_init(VirtIODevice *vdev)
9ae3a8
     }
9ae3a8
 
9ae3a8
     blkconf_serial(&blk->conf, &blk->serial);
9ae3a8
+    s->original_wce = bdrv_enable_write_cache(blk->conf.bs);
9ae3a8
     if (blkconf_geometry(&blk->conf, NULL, 65535, 255, 255) < 0) {
9ae3a8
         return -1;
9ae3a8
     }
9ae3a8
diff --git a/include/hw/virtio/virtio-blk.h b/include/hw/virtio/virtio-blk.h
9ae3a8
index b87cf49..41885da 100644
9ae3a8
--- a/include/hw/virtio/virtio-blk.h
9ae3a8
+++ b/include/hw/virtio/virtio-blk.h
9ae3a8
@@ -123,6 +123,7 @@ typedef struct VirtIOBlock {
9ae3a8
     BlockConf *conf;
9ae3a8
     VirtIOBlkConf blk;
9ae3a8
     unsigned short sector_mask;
9ae3a8
+    bool original_wce;
9ae3a8
     VMChangeStateEntry *change;
9ae3a8
 #ifdef CONFIG_VIRTIO_BLK_DATA_PLANE
9ae3a8
     Notifier migration_state_notifier;
9ae3a8
-- 
9ae3a8
1.7.1
9ae3a8