git: e7fe85643735 - main - xen/blk{front,back}: fix usage of sector sizes different than 512b

From: Roger Pau Monné <royger_at_FreeBSD.org>
Date: Tue, 08 Oct 2024 07:30:01 UTC
The branch main has been updated by royger:

URL: https://cgit.FreeBSD.org/src/commit/?id=e7fe85643735ffdcf18ebef81343eaac9b8d2584

commit e7fe85643735ffdcf18ebef81343eaac9b8d2584
Author:     Roger Pau Monné <royger@FreeBSD.org>
AuthorDate: 2024-08-26 11:57:36 +0000
Commit:     Roger Pau Monné <royger@FreeBSD.org>
CommitDate: 2024-10-08 07:29:13 +0000

    xen/blk{front,back}: fix usage of sector sizes different than 512b
    
    The units of the size reported in the 'sectors' xenbus node is always 512b,
    regardless of the value of the 'sector-size' node.  The sector offsets in
    the ring requests are also always based on 512b sectors, regardless of the
    'sector-size' reported in xenbus.
    
    Fix both blkfront and blkback to assume 512b sectors in the required fields.
    
    The blkif.h public header has been recently updated in upstream Xen repository
    to fix the regressions in the specification introduced by later modifications,
    and clarify the base units of xenstore and shared ring fields.
    
    PR: 280884
    Reported by: Christian Kujau
    MFC after: 1 week
    Sponsored by: Cloud Software Group
    Reviewed by: markj
    Differential revision: https://reviews.freebsd.org/D46756
---
 sys/dev/xen/blkback/blkback.c   | 22 ++++++++++++++-------
 sys/dev/xen/blkfront/blkfront.c | 43 ++++++++++++++++++++++++++++++-----------
 2 files changed, 47 insertions(+), 18 deletions(-)

diff --git a/sys/dev/xen/blkback/blkback.c b/sys/dev/xen/blkback/blkback.c
index 3717264256f3..c6cba729b991 100644
--- a/sys/dev/xen/blkback/blkback.c
+++ b/sys/dev/xen/blkback/blkback.c
@@ -145,6 +145,8 @@ static MALLOC_DEFINE(M_XENBLOCKBACK, "xbbd", "Xen Block Back Driver Data");
  */
 #define	XBB_MAX_SEGMENTS_PER_REQLIST XBB_MAX_SEGMENTS_PER_REQUEST
 
+#define XBD_SECTOR_SHFT		9
+
 /*--------------------------- Forward Declarations ---------------------------*/
 struct xbb_softc;
 struct xbb_xen_req;
@@ -1150,7 +1152,9 @@ xbb_get_resources(struct xbb_softc *xbb, struct xbb_xen_reqlist **reqlist,
 	if (*reqlist == NULL) {
 		*reqlist = nreqlist;
 		nreqlist->operation = ring_req->operation;
-		nreqlist->starting_sector_number = ring_req->sector_number;
+		nreqlist->starting_sector_number =
+		    (ring_req->sector_number << XBD_SECTOR_SHFT) >>
+		    xbb->sector_size_shift;
 		STAILQ_INSERT_TAIL(&xbb->reqlist_pending_stailq, nreqlist,
 				   links);
 	}
@@ -2476,13 +2480,13 @@ xbb_open_file(struct xbb_softc *xbb)
 	xbb->sector_size = 512;
 
 	/*
-	 * Sanity check.  The media size has to be at least one
-	 * sector long.
+	 * Sanity check.  The media size must be a multiple of the sector
+	 * size.
 	 */
-	if (xbb->media_size < xbb->sector_size) {
+	if ((xbb->media_size % xbb->sector_size) != 0) {
 		error = EINVAL;
 		xenbus_dev_fatal(xbb->dev, error,
-				 "file %s size %ju < block size %u",
+				 "file %s size %ju not multiple of block size %u",
 				 xbb->dev_name,
 				 (uintmax_t)xbb->media_size,
 				 xbb->sector_size);
@@ -3086,9 +3090,13 @@ xbb_publish_backend_info(struct xbb_softc *xbb)
 			return (error);
 		}
 
+		/*
+		 * The 'sectors' node is special and always contains the size
+		 * in units of 512b, regardless of the value in 'sector-size'.
+		 */
 		leaf = "sectors";
-		error = xs_printf(xst, our_path, leaf,
-				  "%"PRIu64, xbb->media_num_sectors);
+		error = xs_printf(xst, our_path, leaf, "%ju",
+		    (uintmax_t)(xbb->media_size >> XBD_SECTOR_SHFT));
 		if (error != 0)
 			break;
 
diff --git a/sys/dev/xen/blkfront/blkfront.c b/sys/dev/xen/blkfront/blkfront.c
index b11972295a4b..6bb4e32f1328 100644
--- a/sys/dev/xen/blkfront/blkfront.c
+++ b/sys/dev/xen/blkfront/blkfront.c
@@ -158,7 +158,8 @@ xbd_free_command(struct xbd_command *cm)
 static void
 xbd_mksegarray(bus_dma_segment_t *segs, int nsegs,
     grant_ref_t * gref_head, int otherend_id, int readonly,
-    grant_ref_t * sg_ref, struct blkif_request_segment *sg)
+    grant_ref_t * sg_ref, struct blkif_request_segment *sg,
+    unsigned int sector_size)
 {
 	struct blkif_request_segment *last_block_sg = sg + nsegs;
 	vm_paddr_t buffer_ma;
@@ -166,9 +167,9 @@ xbd_mksegarray(bus_dma_segment_t *segs, int nsegs,
 	int ref;
 
 	while (sg < last_block_sg) {
-		KASSERT(segs->ds_addr % (1 << XBD_SECTOR_SHFT) == 0,
+		KASSERT((segs->ds_addr & (sector_size - 1)) == 0,
 		    ("XEN disk driver I/O must be sector aligned"));
-		KASSERT(segs->ds_len % (1 << XBD_SECTOR_SHFT) == 0,
+		KASSERT((segs->ds_len & (sector_size - 1)) == 0,
 		    ("XEN disk driver I/Os must be a multiple of "
 		    "the sector length"));
 		buffer_ma = segs->ds_addr;
@@ -241,7 +242,8 @@ xbd_queue_cb(void *arg, bus_dma_segment_t *segs, int nsegs, int error)
 		xbd_mksegarray(segs, nsegs, &cm->cm_gref_head,
 		    xenbus_get_otherend_id(sc->xbd_dev),
 		    cm->cm_operation == BLKIF_OP_WRITE,
-		    cm->cm_sg_refs, ring_req->seg);
+		    cm->cm_sg_refs, ring_req->seg,
+		    sc->xbd_disk->d_sectorsize);
 	} else {
 		blkif_request_indirect_t *ring_req;
 
@@ -259,7 +261,8 @@ xbd_queue_cb(void *arg, bus_dma_segment_t *segs, int nsegs, int error)
 		xbd_mksegarray(segs, nsegs, &cm->cm_gref_head,
 		    xenbus_get_otherend_id(sc->xbd_dev),
 		    cm->cm_operation == BLKIF_OP_WRITE,
-		    cm->cm_sg_refs, cm->cm_indirectionpages);
+		    cm->cm_sg_refs, cm->cm_indirectionpages,
+		    sc->xbd_disk->d_sectorsize);
 		memcpy(ring_req->indirect_grefs, &cm->cm_indirectionrefs,
 		    sizeof(grant_ref_t) * sc->xbd_max_request_indirectpages);
 	}
@@ -359,7 +362,9 @@ xbd_bio_command(struct xbd_softc *sc)
 	}
 
 	cm->cm_bp = bp;
-	cm->cm_sector_number = (blkif_sector_t)bp->bio_pblkno;
+	cm->cm_sector_number =
+	    ((blkif_sector_t)bp->bio_pblkno * sc->xbd_disk->d_sectorsize) >>
+	    XBD_SECTOR_SHFT;
 
 	switch (bp->bio_cmd) {
 	case BIO_READ:
@@ -631,7 +636,7 @@ xbd_dump(void *arg, void *virtual, off_t offset, size_t length)
 		cm->cm_data = virtual;
 		cm->cm_datalen = chunk;
 		cm->cm_operation = BLKIF_OP_WRITE;
-		cm->cm_sector_number = offset / dp->d_sectorsize;
+		cm->cm_sector_number = offset >> XBD_SECTOR_SHFT;
 		cm->cm_complete = xbd_dump_complete;
 
 		xbd_enqueue_cm(cm, XBD_Q_READY);
@@ -1025,7 +1030,19 @@ xbd_instance_create(struct xbd_softc *sc, blkif_sector_t sectors,
 	sc->xbd_disk->d_stripesize = phys_sector_size;
 	sc->xbd_disk->d_stripeoffset = 0;
 
-	sc->xbd_disk->d_mediasize = sectors * sector_size;
+	/*
+	 * The 'sectors' xenbus node is always in units of 512b, regardless of
+	 * the 'sector-size' xenbus node value.
+	 */
+	sc->xbd_disk->d_mediasize = sectors << XBD_SECTOR_SHFT;
+	if ((sc->xbd_disk->d_mediasize % sc->xbd_disk->d_sectorsize) != 0) {
+		error = EINVAL;
+		xenbus_dev_fatal(sc->xbd_dev, error,
+		    "Disk size (%ju) not a multiple of sector size (%ju)",
+		    (uintmax_t)sc->xbd_disk->d_mediasize,
+		    (uintmax_t)sc->xbd_disk->d_sectorsize);
+		return (error);
+	}
 	sc->xbd_disk->d_maxsize = sc->xbd_max_request_size;
 	sc->xbd_disk->d_flags = DISKFLAG_UNMAPPED_BIO;
 	if ((sc->xbd_flags & (XBDF_FLUSH|XBDF_BARRIER)) != 0) {
@@ -1310,7 +1327,7 @@ xbd_connect(struct xbd_softc *sc)
 	/* Allocate datastructures based on negotiated values. */
 	err = bus_dma_tag_create(
 	    bus_get_dma_tag(sc->xbd_dev),	/* parent */
-	    512, PAGE_SIZE,			/* algnmnt, boundary */
+	    sector_size, PAGE_SIZE,		/* algnmnt, boundary */
 	    BUS_SPACE_MAXADDR,			/* lowaddr */
 	    BUS_SPACE_MAXADDR,			/* highaddr */
 	    NULL, NULL,				/* filter, filterarg */
@@ -1380,13 +1397,17 @@ xbd_connect(struct xbd_softc *sc)
 
 	if (sc->xbd_disk == NULL) {
 		device_printf(dev, "%juMB <%s> at %s",
-		    (uintmax_t) sectors / (1048576 / sector_size),
+		    (uintmax_t)((sectors << XBD_SECTOR_SHFT) / 1048576),
 		    device_get_desc(dev),
 		    xenbus_get_node(dev));
 		bus_print_child_footer(device_get_parent(dev), dev);
 
-		xbd_instance_create(sc, sectors, sc->xbd_vdevice, binfo,
+		err = xbd_instance_create(sc, sectors, sc->xbd_vdevice, binfo,
 		    sector_size, phys_sector_size);
+		if (err != 0) {
+			xenbus_dev_fatal(dev, err, "Unable to create instance");
+			return;
+		}
 	}
 
 	(void)xenbus_set_state(dev, XenbusStateConnected);