Re: [PATCH 2/5] virtio-blk spec: writeback cache enable improvements

[Rusty Russell <rusty@au1.ibm.com>]

Paolo Bonzini <pbonzini@redhat.com> writes:
> This patch introduces two improvements to writeback cache handling
> in the virtio-blk spec.
>
> 1) The VIRTIO_BLK_F_FLUSH feature is renamed to VIRTIO_BLK_F_WCE, and
> QEMU's behavior is documented explicitly as part of the spec: the host
> negotiates the feature only if its cache is writeback.  The obvious dual
> requirement is imposed on the guest: it should negotiate the feature
> only if it is able to send flushes.  And in order to protect against
> data loss, the spec now mandates that the host operates in writethrough
> mode if the guest does not negotiate VIRTIO_BLK_F_WCE (this behavior
> was already _allowed_ by the spec so far).  This can change with every
> reset of course; typically the BIOS will run as writethrough, while the
> "main" OS will run in writeback mode.  This is a backwards-compatible
> refinement geared towards old or limited guests, so there is no need
> for a new feature bit.
>
> 2) a second feature is added, VIRTIO_BLK_F_CONFIG_WCE, that provides
> the same information in the configuration.  This will enable the driver
> to modify the write-cache setting at runtime (via sysfs for Linux, via
> MODE SELECT for Windows).
>
> Patches for QEMU and Linux will come soonish.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

Here's what it looks like against the text version:

commit 3ee1f0b769c1d9e8ff5edcda11ff52cb41ba0139
Author: Rusty Russell <rusty@au1.ibm.com>
Date:   Wed Aug 21 15:37:33 2013 +0930

    2.5.2: writeback cache enable improvements
    
    From: Paolo Bonzini <pbonzini@redhat.com>
    This patch introduces two improvements to writeback cache handling
    in the virtio-blk spec.
    
    1) The VIRTIO_BLK_F_FLUSH feature is renamed to VIRTIO_BLK_F_WCE, and
    QEMU's behavior is documented explicitly as part of the spec: the host
    negotiates the feature only if its cache is writeback.  The obvious dual
    requirement is imposed on the guest: it should negotiate the feature
    only if it is able to send flushes.  And in order to protect against
    data loss, the spec now mandates that the host operates in writethrough
    mode if the guest does not negotiate VIRTIO_BLK_F_WCE (this behavior
    was already _allowed_ by the spec so far).  This can change with every
    reset of course; typically the BIOS will run as writethrough, while the
    "main" OS will run in writeback mode.  This is a backwards-compatible
    refinement geared towards old or limited guests, so there is no need
    for a new feature bit.
    
    2) a second feature is added, VIRTIO_BLK_F_CONFIG_WCE, that provides
    the same information in the configuration.  This will enable the driver
    to modify the write-cache setting at runtime (via sysfs for Linux, via
    MODE SELECT for Windows).
    
    Patches for QEMU and Linux will come soonish.
    
    Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
    
    See: https://lists.oasis-open.org/archives/virtio-comment/201308/msg00029.html
    
    Signed-off-by: Rusty Russell <rusty@au1.ibm.com>

diff --git a/virtio-v1.0-wd01-part1-specification.txt b/virtio-v1.0-wd01-part1-specification.txt
index f989630..495dd98 100644
--- a/virtio-v1.0-wd01-part1-specification.txt
+++ b/virtio-v1.0-wd01-part1-specification.txt
@@ -1539,11 +1539,16 @@ device except where noted.
 
   VIRTIO_BLK_F_SCSI (7) Device supports scsi packet commands.
 
-  VIRTIO_BLK_F_FLUSH (9) Cache flush command support.
+  VIRTIO_BLK_F_WCE (9) Device cache starts in writeback mode after reset.
+    Guests should not negotiate this feature unless they are capable of sending
+    VIRTIO_BLK_T_FLUSH commands.
 
   VIRTIO_BLK_F_TOPOLOGY (10) Device exports information on optimal I/O
     alignment.
 
+  VIRTIO_BLK_F_CONFIG_WCE (11) Device can toggle its cache between writeback
+    and writethrough modes.
+
   Device configuration layout The capacity of the device 
   (expressed in 512-byte sectors) is always present. The 
   availability of the others all depend on various feature bits 
@@ -1565,6 +1570,7 @@ device except where noted.
 			u16 min_io_size;
 			u32 opt_io_size;
 		} topology;
+		u8 writeback;
 	};
 
 2.5.2.4 Device Initialization
@@ -1588,6 +1594,14 @@ device except where noted.
   I/O lengths for the driver to use. This also does not affect the units
   in the protocol, only performance.
 
+5. The cache mode should be read from the writeback field of the configuration
+  if the VIRTIO_BLK_F_CONFIG_WCE feature if available; the driver can also
+  write to the field in order to toggle the cache between writethrough (0)
+  and writeback (1) mode.
+  If the feature is not available, the driver can instead look at the result
+  of negotiating VIRTIO_BLK_F_WCE: the cache will be in writeback mode after
+  reset if and only if VIRTIO_BLK_F_WCE is negotiated[30]
+
 2.5.2.5 Device Operation
 ------------------------
 
@@ -2885,3 +2899,6 @@ when a port is closed or hot-unplugged.
 
 [29] As updates to configuration space are not atomic, this field
 isn't particularly reliable, but can be used to diagnose buggy guests.
+
+[30] Until version 1.1, QEMU remained in writeback mode even after a guest announced
+ lack of support for VIRTIO_BLK_F_FLUSH.