Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 15 Oct 2024 08:26:36 GMT
From:      Roger Pau =?utf-8?Q?Monn=C3=A9?= <royger@FreeBSD.org>
To:        src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org
Subject:   git: 9da7b206f9d6 - stable/13 - xen/blk{front,back}: fix usage of sector sizes different than 512b
Message-ID:  <202410150826.49F8QaXu009676@gitrepo.freebsd.org>

next in thread | raw e-mail | index | archive | help
The branch stable/13 has been updated by royger:

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

commit 9da7b206f9d673a81574d6d08ec430ab49599735
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-15 08:14:59 +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
    
    (cherry picked from commit e7fe85643735ffdcf18ebef81343eaac9b8d2584)
---
 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 a0ac8e13ec71..c58af5e9c90f 100644
--- a/sys/dev/xen/blkback/blkback.c
+++ b/sys/dev/xen/blkback/blkback.c
@@ -166,6 +166,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;
@@ -1226,7 +1228,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);
 	}
@@ -2633,13 +2637,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);
@@ -3260,9 +3264,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 90b5130273b4..e1fcf378316d 100644
--- a/sys/dev/xen/blkfront/blkfront.c
+++ b/sys/dev/xen/blkfront/blkfront.c
@@ -160,7 +160,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;
@@ -168,9 +169,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;
@@ -243,7 +244,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;
 
@@ -261,7 +263,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);
 	}
@@ -361,7 +364,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:
@@ -633,7 +638,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);
@@ -1027,7 +1032,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) {
@@ -1314,7 +1331,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 */
@@ -1386,13 +1403,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); 



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202410150826.49F8QaXu009676>