All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
From: Michael Roth <mdroth@linux.vnet.ibm.com>
To: qemu-devel@nongnu.org
Cc: qemu-stable@nongnu.org
Subject: [Qemu-devel] [PATCH 38/38] virtio-blk: do not relay a previous driver's WCE configuration to the current
Date: Wed, 25 Sep 2013 07:58:06 -0500	[thread overview]
Message-ID: <1380113886-16845-39-git-send-email-mdroth@linux.vnet.ibm.com> (raw)
In-Reply-To: <1380113886-16845-1-git-send-email-mdroth@linux.vnet.ibm.com>

From: Paolo Bonzini <pbonzini@redhat.com>

The following sequence happens:
- the SeaBIOS virtio-blk driver does not support the WCE feature, which
causes QEMU to disable writeback caching

- the Linux virtio-blk driver resets the device, finds WCE is available
but writeback caching is disabled; tells block layer to not send cache
flush commands

- the Linux virtio-blk driver sets the DRIVER_OK bit, which causes
writeback caching to be re-enabled, but the Linux virtio-blk driver does
not know of this side effect and cache flushes remain disabled

The bug is at the third step.  If the guest does know about CONFIG_WCE,
QEMU should ignore the WCE feature's state.  The guest will control the
cache mode solely using configuration space.  This change makes Linux
do flushes correctly, but Linux will keep SeaBIOS's writethrough mode.

Hence, whenever the guest is reset, the cache mode of the disk should
be reset to whatever was specified in the "-drive" option.  With this
change, the Linux virtio-blk driver finds that writeback caching is
enabled, and tells the block layer to send cache flush commands
appropriately.

Reported-by: Rusty Russell <rusty@au1.ibm.com
Cc: qemu-stable@nongnu.org
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
(cherry picked from commit ef5bc96268ceec64769617dc53b0ac3a20ff351c)

Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 hw/block/virtio-blk.c          |   24 ++++++++++++++++++++++--
 include/hw/virtio/virtio-blk.h |    1 +
 2 files changed, 23 insertions(+), 2 deletions(-)

diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index e2f55cc..49a23c3 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -460,9 +460,9 @@ static void virtio_blk_dma_restart_cb(void *opaque, int running,
 
 static void virtio_blk_reset(VirtIODevice *vdev)
 {
-#ifdef CONFIG_VIRTIO_BLK_DATA_PLANE
     VirtIOBlock *s = VIRTIO_BLK(vdev);
 
+#ifdef CONFIG_VIRTIO_BLK_DATA_PLANE
     if (s->dataplane) {
         virtio_blk_data_plane_stop(s->dataplane);
     }
@@ -473,6 +473,7 @@ static void virtio_blk_reset(VirtIODevice *vdev)
      * are per-device request lists.
      */
     bdrv_drain_all();
+    bdrv_set_enable_write_cache(s->bs, s->original_wce);
 }
 
 /* coalesce internal state, copy to pci i/o region 0
@@ -564,7 +565,25 @@ static void virtio_blk_set_status(VirtIODevice *vdev, uint8_t status)
     }
 
     features = vdev->guest_features;
-    bdrv_set_enable_write_cache(s->bs, !!(features & (1 << VIRTIO_BLK_F_WCE)));
+
+    /* A guest that supports VIRTIO_BLK_F_CONFIG_WCE must be able to send
+     * cache flushes.  Thus, the "auto writethrough" behavior is never
+     * necessary for guests that support the VIRTIO_BLK_F_CONFIG_WCE feature.
+     * Leaving it enabled would break the following sequence:
+     *
+     *     Guest started with "-drive cache=writethrough"
+     *     Guest sets status to 0
+     *     Guest sets DRIVER bit in status field
+     *     Guest reads host features (WCE=0, CONFIG_WCE=1)
+     *     Guest writes guest features (WCE=0, CONFIG_WCE=1)
+     *     Guest writes 1 to the WCE configuration field (writeback mode)
+     *     Guest sets DRIVER_OK bit in status field
+     *
+     * s->bs would erroneously be placed in writethrough mode.
+     */
+    if (!(features & (1 << VIRTIO_BLK_F_CONFIG_WCE))) {
+        bdrv_set_enable_write_cache(s->bs, !!(features & (1 << VIRTIO_BLK_F_WCE)));
+    }
 }
 
 static void virtio_blk_save(QEMUFile *f, void *opaque)
@@ -674,6 +693,7 @@ static int virtio_blk_device_init(VirtIODevice *vdev)
     }
 
     blkconf_serial(&blk->conf, &blk->serial);
+    s->original_wce = bdrv_enable_write_cache(blk->conf.bs);
     if (blkconf_geometry(&blk->conf, NULL, 65535, 255, 255) < 0) {
         return -1;
     }
diff --git a/include/hw/virtio/virtio-blk.h b/include/hw/virtio/virtio-blk.h
index b87cf49..41885da 100644
--- a/include/hw/virtio/virtio-blk.h
+++ b/include/hw/virtio/virtio-blk.h
@@ -123,6 +123,7 @@ typedef struct VirtIOBlock {
     BlockConf *conf;
     VirtIOBlkConf blk;
     unsigned short sector_mask;
+    bool original_wce;
     VMChangeStateEntry *change;
 #ifdef CONFIG_VIRTIO_BLK_DATA_PLANE
     Notifier migration_state_notifier;
-- 
1.7.9.5

  parent reply	other threads:[~2013-09-25 12:59 UTC|newest]

Thread overview: 51+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-09-25 12:57 [Qemu-devel] Patch Round-up for stable 1.6.1, freeze on 2013-09-30 Michael Roth
2013-09-25 12:57 ` [Qemu-devel] [PATCH 01/38] block: ensure bdrv_drain_all() works during bdrv_delete() Michael Roth
2013-09-25 12:57 ` [Qemu-devel] [PATCH 02/38] gdbstub: Fix gdb_register_coprocessor() register counting Michael Roth
2013-09-25 12:57 ` [Qemu-devel] [PATCH 03/38] target-ppc: fix bit extraction for FPBF and FPL Michael Roth
2013-09-25 12:57 ` [Qemu-devel] [PATCH 04/38] rdma: silly ipv6 bugfix Michael Roth
2013-09-25 12:57 ` [Qemu-devel] [PATCH 05/38] scripts/qapi.py: Avoid syntax not supported by Python 2.4 Michael Roth
2013-09-25 12:57 ` [Qemu-devel] [PATCH 06/38] usb/dev-hid: Modified usb-tablet category from Misc to Input Michael Roth
2013-09-25 12:57 ` [Qemu-devel] [PATCH 07/38] scsi: Fix scsi_bus_legacy_add_drive() scsi-generic with serial Michael Roth
2013-09-25 12:57 ` [Qemu-devel] [PATCH 08/38] pc: fix regression for 64 bit PCI memory Michael Roth
2013-09-25 12:57 ` [Qemu-devel] [PATCH 09/38] pseries: Fix stalls on hypervisor virtual console Michael Roth
2013-09-25 12:57 ` [Qemu-devel] [PATCH 10/38] virtio: virtqueue_get_avail_bytes: fix desc_pa when loop over the indirect descriptor table Michael Roth
2013-09-25 12:57 ` [Qemu-devel] [PATCH 11/38] xhci: fix endpoint interval calculation Michael Roth
2013-09-25 13:41   ` Gerd Hoffmann
2013-09-25 12:57 ` [Qemu-devel] [PATCH 12/38] Revert "usb-hub: report status changes only once" Michael Roth
2013-09-25 12:57 ` [Qemu-devel] [PATCH 13/38] block: expect errors from bdrv_co_is_allocated Michael Roth
2013-09-25 21:27   ` [Qemu-devel] [Qemu-stable] " Doug Goldstein
2013-09-26 20:51     ` Paolo Bonzini
2013-09-25 12:57 ` [Qemu-devel] [PATCH 14/38] target-i386: fix disassembly with PAE=1, PG=0 Michael Roth
2013-09-25 12:57 ` [Qemu-devel] [PATCH 15/38] adlib: sort offsets in portio registration Michael Roth
2013-09-25 12:57 ` [Qemu-devel] [PATCH 16/38] exec: fix writing to MMIO area with non-power-of-two length Michael Roth
2013-09-25 12:57 ` [Qemu-devel] [PATCH 17/38] virtio_pci: fix level interrupts with irqfd Michael Roth
2013-09-25 12:57 ` [Qemu-devel] [PATCH 18/38] exec: always use MADV_DONTFORK Michael Roth
2013-09-25 12:57 ` [Qemu-devel] [PATCH 19/38] xhci: reset port when disabling slot Michael Roth
2013-09-25 12:57 ` [Qemu-devel] [PATCH 20/38] usb: parallelize usb3 streams Michael Roth
2013-09-25 12:57 ` [Qemu-devel] [PATCH 21/38] w32: Fix access to host devices (regression) Michael Roth
2013-09-25 12:57 ` [Qemu-devel] [PATCH 22/38] memory: Provide separate handling of unassigned io ports accesses Michael Roth
2013-09-25 12:57 ` [Qemu-devel] [PATCH 23/38] Revert "memory: Return -1 again on reads from unsigned regions" Michael Roth
2013-09-25 12:57 ` [Qemu-devel] [PATCH 24/38] exec: check offset_within_address_space for register subpage Michael Roth
2013-09-25 12:57 ` [Qemu-devel] [PATCH 25/38] ne2000: mark I/O as LITTLE_ENDIAN Michael Roth
2013-09-25 12:57 ` [Qemu-devel] [PATCH 26/38] ehci: save device pointer in EHCIState Michael Roth
2013-09-25 12:57 ` [Qemu-devel] [PATCH 27/38] qxl: fix local renderer Michael Roth
2013-09-25 12:57 ` [Qemu-devel] [PATCH 28/38] pc: Initializing ram_memory under Xen Michael Roth
2013-09-25 12:57 ` [Qemu-devel] [PATCH 29/38] pc_q35: Initialize Xen Michael Roth
2013-09-25 12:57 ` [Qemu-devel] [PATCH 30/38] qapi-types.py: Fix enum struct sizes on i686 Michael Roth
2013-09-25 12:57 ` [Qemu-devel] [PATCH 31/38] pcnet-pci: mark I/O and MMIO as LITTLE_ENDIAN Michael Roth
2013-09-25 12:58 ` [Qemu-devel] [PATCH 32/38] chardev: fix pty_chr_timer Michael Roth
2013-09-25 12:58 ` [Qemu-devel] [PATCH 33/38] kvmvapic: Catch invalid ROM size Michael Roth
2013-09-25 12:58 ` [Qemu-devel] [PATCH 34/38] kvmvapic: Enter inactive state on hardware reset Michael Roth
2013-09-25 12:58 ` [Qemu-devel] [PATCH 35/38] kvmvapic: Clear also physical ROM address when entering INACTIVE state Michael Roth
2013-09-25 12:58 ` [Qemu-devel] [PATCH 36/38] tci: Fix qemu-alpha on 32 bit hosts (wrong assertions) Michael Roth
2013-09-25 12:58 ` [Qemu-devel] [PATCH 37/38] blockdev: do not default cache.no-flush to true Michael Roth
2013-09-25 12:58 ` Michael Roth [this message]
2013-09-25 13:54 ` [Qemu-devel] [Qemu-stable] Patch Round-up for stable 1.6.1, freeze on 2013-09-30 Cole Robinson
2013-09-25 21:43   ` Doug Goldstein
2013-09-26 20:53   ` Paolo Bonzini
2013-09-26  4:53 ` [Qemu-devel] " Stefan Weil
2013-09-26  5:22 ` Fam Zheng
2013-09-26 19:38 ` Stefan Weil
2013-09-27  7:30   ` Michael Tokarev
2013-09-27  8:07 ` [Qemu-devel] [Qemu-stable] " Michael Tokarev
2013-09-27  8:11   ` Michael Tokarev

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=1380113886-16845-39-git-send-email-mdroth@linux.vnet.ibm.com \
    --to=mdroth@linux.vnet.ibm.com \
    --cc=qemu-devel@nongnu.org \
    --cc=qemu-stable@nongnu.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.