Re: xbd and large sectors

[Manuel Bouyer <bouyer%antioche.eu.org@localhost>]

On Mon, Jul 24, 2023 at 03:33:09PM +0200, Christian Kujau wrote:
> On Mon, 24 Jul 2023, Manuel Bouyer wrote:
> > But this is not expected, the request looks correct (it's aligned and
> > size is a multiple of 8 DEV_BSIZE units).
> > 
> > Does anything shows up in the dom0's dmesg ?
> 
> No errors, only the usual messages when starting the domU:

OK, so here's an updated diff and I also updated the binaries.
This time I enabled XBD_DEBUG
please try and report the boot messages

-- 
Manuel Bouyer <bouyer%antioche.eu.org@localhost>
     NetBSD: 26 ans d'experience feront toujours la difference
--

Index: sys/arch/xen/xen/xbd_xenbus.c
===================================================================
RCS file: /cvsroot/src/sys/arch/xen/xen/xbd_xenbus.c,v
retrieving revision 1.133
diff -u -p -u -r1.133 xbd_xenbus.c
--- sys/arch/xen/xen/xbd_xenbus.c	21 Jul 2023 11:28:50 -0000	1.133
+++ sys/arch/xen/xen/xbd_xenbus.c	24 Jul 2023 13:52:18 -0000
@@ -85,6 +85,7 @@ __KERNEL_RCSID(0, "$NetBSD: xbd_xenbus.c
 #include "locators.h"
 
 #undef XBD_DEBUG
+#define XBD_DEBUG
 #ifdef XBD_DEBUG
 #define DPRINTF(x) printf x;
 #else
@@ -169,7 +170,7 @@ struct xbd_xenbus_softc {
 #define BLKIF_SHUTDOWN_REMOTE 1 /* backend-initiated shutdown in progress */
 #define BLKIF_SHUTDOWN_LOCAL  2 /* locally-initiated shutdown in progress */
 
-	uint64_t sc_sectors; /* number of XEN_BSIZE sectors for this device */
+	uint64_t sc_sectors; /* number of sc_secsize sectors for this device */
 	u_long sc_secsize; /* sector size */
 	uint64_t sc_xbdsize; /* size of disk in DEV_BSIZE */
 	u_long sc_info; /* VDISK_* */
@@ -674,15 +675,14 @@ xbd_backend_changed(void *arg, XenbusSta
 		xbd_connect(sc);
 		sc->sc_shutdown = BLKIF_SHUTDOWN_RUN;
 		sc->sc_xbdsize =
-		    sc->sc_sectors * (uint64_t)XEN_BSIZE / DEV_BSIZE;
+		    sc->sc_sectors * (uint64_t)sc->sc_secsize / DEV_BSIZE;
 		dg = &sc->sc_dksc.sc_dkdev.dk_geom;
 		memset(dg, 0, sizeof(*dg));
 
-		dg->dg_secperunit = sc->sc_xbdsize;
-		dg->dg_secsize = DEV_BSIZE;
+		dg->dg_secperunit = sc->sc_sectors;
+		dg->dg_secsize = sc->sc_secsize;
 		dg->dg_ntracks = 1;
-		// XXX: Ok to hard-code DEV_BSIZE?
-		dg->dg_nsectors = 1024 * (1024 / dg->dg_secsize);
+		dg->dg_nsectors = (1024 * 1024) / dg->dg_secsize;
 		dg->dg_ncylinders = dg->dg_secperunit / dg->dg_nsectors;
 
 		bufq_alloc(&sc->sc_dksc.sc_bufq, "fcfs", 0);
@@ -693,10 +693,10 @@ xbd_backend_changed(void *arg, XenbusSta
 		hypervisor_unmask_event(sc->sc_evtchn);
 
 		format_bytes(buf, uimin(9, sizeof(buf)),
-		    sc->sc_sectors * XEN_BSIZE);
+		    sc->sc_sectors * dg->dg_secsize);
 		aprint_normal_dev(sc->sc_dksc.sc_dev,
 				"%s, %d bytes/sect x %" PRIu64 " sectors\n",
-				buf, (int)dg->dg_secsize, sc->sc_xbdsize);
+				buf, (int)dg->dg_secsize, sc->sc_sectors);
 		snprintb(buf, sizeof(buf), BLKIF_FEATURE_BITS,
 		    sc->sc_features);
 		aprint_normal_dev(sc->sc_dksc.sc_dev,
@@ -739,14 +739,6 @@ xbd_connect(struct xbd_xenbus_softc *sc)
 		panic("%s: can't read number from %s/virtual-device\n",
 		    device_xname(sc->sc_dksc.sc_dev),
 		    sc->sc_xbusd->xbusd_otherend);
-	err = xenbus_read_ull(NULL,
-	    sc->sc_xbusd->xbusd_otherend, "sectors", &sectors, 10);
-	if (err)
-		panic("%s: can't read number from %s/sectors\n",
-		    device_xname(sc->sc_dksc.sc_dev),
-		    sc->sc_xbusd->xbusd_otherend);
-	sc->sc_sectors = sectors;
-
 	err = xenbus_read_ul(NULL,
 	    sc->sc_xbusd->xbusd_otherend, "info", &sc->sc_info, 10);
 	if (err)
@@ -760,6 +752,14 @@ xbd_connect(struct xbd_xenbus_softc *sc)
 		    device_xname(sc->sc_dksc.sc_dev),
 		    sc->sc_xbusd->xbusd_otherend);
 
+	err = xenbus_read_ull(NULL,
+	    sc->sc_xbusd->xbusd_otherend, "sectors", &sectors, 10);
+	if (err)
+		panic("%s: can't read number from %s/sectors\n",
+		    device_xname(sc->sc_dksc.sc_dev),
+		    sc->sc_xbusd->xbusd_otherend);
+	sc->sc_sectors = sectors * (uint64_t)XEN_BSIZE / sc->sc_secsize;
+
 	xenbus_switch_state(sc->sc_xbusd, NULL, XenbusStateConnected);
 }
 
@@ -1274,7 +1274,8 @@ xbd_diskstart_submit(struct xbd_xenbus_s
 	req->id = req_id;
 	req->operation =
 	    bp->b_flags & B_READ ? BLKIF_OP_READ : BLKIF_OP_WRITE;
-	req->sector_number = bp->b_rawblkno + (start >> XEN_BSHIFT);
+	req->sector_number = (bp->b_rawblkno * DEV_BSIZE / XEN_BSIZE) +
+	    (start >> XEN_BSHIFT);
 	req->handle = sc->sc_handle;
 
 	size = uimin(bp->b_bcount - start, XBD_MAX_CHUNK);
@@ -1331,7 +1332,7 @@ xbd_diskstart_submit_indirect(struct xbd
 	req->operation = BLKIF_OP_INDIRECT;
 	req->indirect_op =
 	    bp->b_flags & B_READ ? BLKIF_OP_READ : BLKIF_OP_WRITE;
-	req->sector_number = bp->b_rawblkno;
+	req->sector_number = bp->b_rawblkno * DEV_BSIZE / XEN_BSIZE;
 	req->handle = sc->sc_handle;
 
 	xbdreq->req_indirect = SLIST_FIRST(&sc->sc_indirect_head);