[PATCH 0/2] virtio-blk: improve zone write granularity handling

[PATCH 0/2] virtio-blk: improve zone write granularity handling

[Niklas Cassel]

Hello Stefan,

A small series which improves the zone write granularity handling in
virtio-blk.

Please review.

Kind regards,
Niklas


Niklas Cassel (2):
  virtio-blk: report the effective zone write granularity
  virtio-blk: check the write granularity of writes to sequential zones

 hw/block/virtio-blk.c | 47 +++++++++++++++++++++++++++++++++++++------
 1 file changed, 41 insertions(+), 6 deletions(-)

-- 
2.55.0

[PATCH 1/2] virtio-blk: report the effective zone write granularity

[Niklas Cassel]

For ZBC/ZAC devices the write granularity is the physical block size, so
a 512e SMR disk exposed through a host_device backend has a logical block
size of 512 and a zone write granularity of 4096. We told the driver 512
while raw_co_zone_append() rejects anything that is not 4096 byte
aligned, so the driver saw a plain I/O error for a request it had been
told was valid.

Report the larger of the backend granularity and the logical block size
instead. The driver cannot issue writes finer than the logical block
size, so the larger of the two is the constraint that applies. This
matches what a Linux guest derives for itself: blk_validate_zoned_limits()
raises zone_write_granularity to the logical block size, and
blk_stack_limits() stacks it with max().

Add it as a helper and use it for the zone append offset check in
check_zoned_request(), which validated against bs->bl.write_granularity,
so that the value reported to the driver and the value that requests are
validated against cannot drift apart. The helper cannot return zero
because blkconf_blocksizes() always leaves a logical block size behind,
so the check no longer needs to guard against an unset granularity.

Fixes: 4f7366506a96 ("virtio-blk: add zoned storage emulation for zoned devices")
Signed-off-by: Niklas Cassel <cassel@kernel.org>
---
 hw/block/virtio-blk.c | 25 +++++++++++++++++++------
 1 file changed, 19 insertions(+), 6 deletions(-)

diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index 6b92066aff..ef5581c1e6 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -478,6 +478,18 @@ typedef struct ZoneCmdData {
     };
 } ZoneCmdData;
 
+/*
+ * The write granularity that the device reports to the driver in
+ * virtio_blk_zoned_characteristics: the offset and size alignment constraint
+ * for writes and zone appends to sequential zones.
+ */
+static uint32_t virtio_blk_write_granularity(VirtIOBlock *s)
+{
+    BlockDriverState *bs = blk_bs(s->blk);
+
+    return MAX(bs->bl.write_granularity, s->conf.conf.logical_block_size);
+}
+
 /*
  * check zoned_request: error checking before issuing requests. If all checks
  * passed, return true.
@@ -500,11 +512,11 @@ static bool check_zoned_request(VirtIOBlock *s, int64_t offset, int64_t len,
     }
 
     if (append) {
-        if (bs->bl.write_granularity) {
-            if ((offset % bs->bl.write_granularity) != 0) {
-                *status = VIRTIO_BLK_S_ZONE_UNALIGNED_WP;
-                return false;
-            }
+        uint32_t wg = virtio_blk_write_granularity(s);
+
+        if ((offset % wg) != 0) {
+            *status = VIRTIO_BLK_S_ZONE_UNALIGNED_WP;
+            return false;
         }
 
         index = offset / bs->bl.zone_size;
@@ -1254,7 +1266,8 @@ static void virtio_blk_update_config(VirtIODevice *vdev, uint8_t *config)
                      bs->bl.max_active_zones);
         virtio_stl_p(vdev, &blkcfg.zoned.max_open_zones,
                      bs->bl.max_open_zones);
-        virtio_stl_p(vdev, &blkcfg.zoned.write_granularity, blk_size);
+        virtio_stl_p(vdev, &blkcfg.zoned.write_granularity,
+                     virtio_blk_write_granularity(s));
         virtio_stl_p(vdev, &blkcfg.zoned.max_append_sectors,
                      bs->bl.max_append_sectors);
     } else {
-- 
2.55.0

[PATCH 2/2] virtio-blk: check the write granularity of writes to sequential zones

[Niklas Cassel]

All VIRTIO_BLK_T_OUT requests issued to sequential zones and all
VIRTIO_BLK_T_ZONE_APPEND requests must have an offset and a data size
that are multiples of the write granularity reported by the device
(virtio 1.4, 5.2.6.1), and a violation is reported as
VIRTIO_BLK_S_ZONE_UNALIGNED_WP (virtio 1.4, 5.2.6).

Neither request type was fully checked. Zone appends validated only the
offset, while writes were not checked at all.

Check the size of the appended data, and both the offset and the size of
a write, against virtio_blk_write_granularity(), so that every request
the device accepts is one that the driver was told is valid. Writes to
conventional zones keep no alignment constraint beyond the logical block
size. The write path performs the check after virtio_blk_sect_range_ok()
so that the zone index derived from the guest supplied sector is known to
be in range.

Signed-off-by: Niklas Cassel <cassel@kernel.org>
---
 hw/block/virtio-blk.c | 24 +++++++++++++++++++++++-
 1 file changed, 23 insertions(+), 1 deletion(-)

diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index ef5581c1e6..b1be0e9eda 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -514,7 +514,7 @@ static bool check_zoned_request(VirtIOBlock *s, int64_t offset, int64_t len,
     if (append) {
         uint32_t wg = virtio_blk_write_granularity(s);
 
-        if ((offset % wg) != 0) {
+        if ((offset % wg) != 0 || (len % wg) != 0) {
             *status = VIRTIO_BLK_S_ZONE_UNALIGNED_WP;
             return false;
         }
@@ -903,6 +903,28 @@ static int virtio_blk_handle_request(VirtIOBlockReq *req, MultiReqBuffer *mrb)
             return 0;
         }
 
+        if (is_write) {
+            BlockDriverState *bs = blk_bs(s->blk);
+            int64_t offset = req->sector_num << BDRV_SECTOR_BITS;
+            uint32_t wg = virtio_blk_write_granularity(s);
+
+            /*
+             * Both the offset and the size of a write to a sequential zone
+             * must be a multiple of the write granularity reported by the
+             * device. Conventional zones are not constrained. The zone index
+             * is derived from a guest supplied sector, so this must come after
+             * virtio_blk_sect_range_ok() has bounded it.
+             */
+            if (bs->bl.zoned != BLK_Z_NONE &&
+                ((offset % wg) != 0 || (req->qiov.size % wg) != 0) &&
+                !BDRV_ZT_IS_CONV(bs->wps->wp[offset / bs->bl.zone_size])) {
+                virtio_blk_req_complete(req, VIRTIO_BLK_S_ZONE_UNALIGNED_WP);
+                block_acct_invalid(blk_get_stats(s->blk), BLOCK_ACCT_WRITE);
+                g_free(req);
+                return 0;
+            }
+        }
+
         block_acct_start(blk_get_stats(s->blk), &req->acct, req->qiov.size,
                          is_write ? BLOCK_ACCT_WRITE : BLOCK_ACCT_READ);
 
-- 
2.55.0