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