[PATCH 2/2] hw/block/xen-block: Update sector-size handling

[Anthony PERARD <anthony.perard@xxxxxxxxxx>]

The use of "feature-large-sector-size" have been removed from the
protocol, as it hasn't been evenly implemented across all backend and
frontend. Linux for example will happily expose "sector-size" != 512
even when "feature-large-sector-size" hasn't been set by the frontend.

The specification have been clarified regarding what "sector" is by
Xen commit 221f2748e8da ("blkif: reconcile protocol specification with
in-use implementations").

https://xenbits.xenproject.org/gitweb/?p=xen.git;a=commit;h=221f2748e8dabe8361b8cdfcffbeab9102c4c899

So change QEMU to follow the updated specification.

Frontends that exposes "feature-large-sector-size" will most certainly
misbehave if "sector-size" is different than 512, so don't even try.
(Windows driver is likely to be the only one having implemented it.)

Signed-off-by: Anthony PERARD <anthony.perard@xxxxxxxxxx>
---
 hw/block/dataplane/xen-block.c | 31 ++++++++++++++++++++++---------
 hw/block/xen-block.c           |  8 ++++----
 2 files changed, 26 insertions(+), 13 deletions(-)

diff --git a/hw/block/dataplane/xen-block.c b/hw/block/dataplane/xen-block.c
index 98501e6885..43be97b4f3 100644
--- a/hw/block/dataplane/xen-block.c
+++ b/hw/block/dataplane/xen-block.c
@@ -176,7 +176,11 @@ static int xen_block_parse_request(XenBlockRequest 
*request)
         goto err;
     }
 
-    request->start = request->req.sector_number * dataplane->sector_size;
+    request->start = request->req.sector_number * XEN_BLKIF_SECTOR_SIZE;
+    if (!QEMU_IS_ALIGNED(request->start, dataplane->sector_size)) {
+        error_report("error: sector_number missaligned with sector-size");
+        goto err;
+    }
     for (i = 0; i < request->req.nr_segments; i++) {
         if (i == BLKIF_MAX_SEGMENTS_PER_REQUEST) {
             error_report("error: nr_segments too big");
@@ -186,14 +190,23 @@ static int xen_block_parse_request(XenBlockRequest 
*request)
             error_report("error: first > last sector");
             goto err;
         }
-        if (request->req.seg[i].last_sect * dataplane->sector_size >=
+        if (request->req.seg[i].last_sect * XEN_BLKIF_SECTOR_SIZE >=
             XEN_PAGE_SIZE) {
             error_report("error: page crossing");
             goto err;
         }
+        if (!QEMU_IS_ALIGNED(request->req.seg[i].first_sect,
+                             dataplane->sector_size / XEN_BLKIF_SECTOR_SIZE)) {
+            error_report("error: first_sect missaligned with sector-size");
+            goto err;
+        }
 
         len = (request->req.seg[i].last_sect -
-               request->req.seg[i].first_sect + 1) * dataplane->sector_size;
+               request->req.seg[i].first_sect + 1) * XEN_BLKIF_SECTOR_SIZE;
+        if (!QEMU_IS_ALIGNED(len, dataplane->sector_size)) {
+            error_report("error: segment size missaligned with sector-size");
+            goto err;
+        }
         request->size += len;
     }
     if (request->start + request->size > blk_getlength(dataplane->blk)) {
@@ -227,17 +240,17 @@ static int xen_block_copy_request(XenBlockRequest 
*request)
         if (to_domain) {
             segs[i].dest.foreign.ref = request->req.seg[i].gref;
             segs[i].dest.foreign.offset = request->req.seg[i].first_sect *
-                dataplane->sector_size;
+                XEN_BLKIF_SECTOR_SIZE;
             segs[i].source.virt = virt;
         } else {
             segs[i].source.foreign.ref = request->req.seg[i].gref;
             segs[i].source.foreign.offset = request->req.seg[i].first_sect *
-                dataplane->sector_size;
+                XEN_BLKIF_SECTOR_SIZE;
             segs[i].dest.virt = virt;
         }
         segs[i].len = (request->req.seg[i].last_sect -
                        request->req.seg[i].first_sect + 1) *
-                      dataplane->sector_size;
+                      XEN_BLKIF_SECTOR_SIZE;
         virt += segs[i].len;
     }
 
@@ -331,12 +344,12 @@ static bool xen_block_split_discard(XenBlockRequest 
*request,
 
     /* Wrap around, or overflowing byte limit? */
     if (sec_start + sec_count < sec_count ||
-        sec_start + sec_count > INT64_MAX / dataplane->sector_size) {
+        sec_start + sec_count > INT64_MAX / XEN_BLKIF_SECTOR_SIZE) {
         return false;
     }
 
-    byte_offset = sec_start * dataplane->sector_size;
-    byte_remaining = sec_count * dataplane->sector_size;
+    byte_offset = sec_start * XEN_BLKIF_SECTOR_SIZE;
+    byte_remaining = sec_count * XEN_BLKIF_SECTOR_SIZE;
 
     do {
         byte_chunk = byte_remaining > BDRV_REQUEST_MAX_BYTES ?
diff --git a/hw/block/xen-block.c b/hw/block/xen-block.c
index aed1d5c330..8c150c6c69 100644
--- a/hw/block/xen-block.c
+++ b/hw/block/xen-block.c
@@ -189,10 +189,10 @@ static void xen_block_connect(XenDevice *xendev, Error 
**errp)
         feature_large_sector_size = 0;
     }
 
-    if (feature_large_sector_size != 1 &&
+    if (feature_large_sector_size == 1 &&
         conf->logical_block_size != XEN_BLKIF_SECTOR_SIZE) {
-        error_setg(errp, "logical_block_size != %u not supported by frontend",
-                   XEN_BLKIF_SECTOR_SIZE);
+        error_setg(errp,
+                   "\"feature-large-sector-size\" not supported by backend");
         return;
     }
 
@@ -294,7 +294,7 @@ static void xen_block_set_size(XenBlockDevice *blockdev)
     const char *type = object_get_typename(OBJECT(blockdev));
     XenBlockVdev *vdev = &blockdev->props.vdev;
     BlockConf *conf = &blockdev->props.conf;
-    int64_t sectors = blk_getlength(conf->blk) / conf->logical_block_size;
+    int64_t sectors = blk_getlength(conf->blk) / XEN_BLKIF_SECTOR_SIZE;
     XenDevice *xendev = XEN_DEVICE(blockdev);
 
     trace_xen_block_size(type, vdev->disk, vdev->partition, sectors);
-- 


Anthony Perard | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech