Bounce unaligned transfer data through a scratch buffer

Thanks to the detailed report from Nathanael Rensen, the issue
with unaligned transfer data became apparent: the backend expects
buffers be multiple of 512 bytes and to be 512 byte aligned, which
is not always satisfied.

This isn't an issue when requests are coming from the buffer cache,
but can happen with raw device access since physio(9) ensures the
former requirement is met by disallowing non-block sized reads, but
doesn't enforce the latter.  It remaps userland buffers into the
kernel virtual space which preserves the data offset within the
memory page and thus the original alignment.

Buffers with offsets under the block size can't be referenced by
Blkfront ring descriptors that measure data in blocks and must be
substituted with temporary buffers for the duration of the I/O
operation.

1 files changed, 186 insertions, 46 deletions

diff --git a/sys/dev/pv/xbf.c b/sys/dev/pv/xbf.c
index ae521e25055..50f1c11c5e2 100644
--- a/sys/dev/pv/xbf.c
+++ b/sys/dev/pv/xbf.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: xbf.c,v 1.10 2016/12/13 18:27:24 mikeb Exp $	*/
+/*	$OpenBSD: xbf.c,v 1.11 2016/12/13 19:02:39 mikeb Exp $	*/
 
 /*
  * Copyright (c) 2016 Mike Belopuhov
@@ -128,8 +128,9 @@ struct xbf_dma_mem {
 	bus_size_t		 dma_size;
 	bus_dma_tag_t		 dma_tag;
 	bus_dmamap_t		 dma_map;
-	bus_dma_segment_t	 dma_seg;
-	int			 dma_nsegs;
+	bus_dma_segment_t	*dma_seg;
+	int			 dma_nsegs; /* total amount */
+	int			 dma_rsegs; /* used amount */
 	caddr_t			 dma_vaddr;
 };
 
@@ -170,6 +171,7 @@ struct xbf_softc {
 	struct scsi_xfer	**sc_xs;
 	bus_dmamap_t		*sc_xs_map;
 	int			 sc_xs_avail;
+	struct xbf_dma_mem	*sc_xs_bb;
 
 	struct scsi_iopool	 sc_iopool;
 	struct scsi_adapter	 sc_switch;
@@ -193,6 +195,10 @@ void	xbf_intr(void *);
 void	*xbf_io_get(void *);
 void	xbf_io_put(void *, void *);
 
+int	xbf_load_xs(struct scsi_xfer *, int);
+int	xbf_bounce_xs(struct scsi_xfer *, int);
+void	xbf_reclaim_xs(struct scsi_xfer *, int);
+
 void	xbf_scsi_cmd(struct scsi_xfer *);
 int	xbf_submit_cmd(struct scsi_xfer *);
 int	xbf_poll_cmd(struct scsi_xfer *, int, int);
@@ -400,17 +406,144 @@ xbf_scsi_cmd(struct scsi_xfer *xs)
 	if (ISSET(xs->flags, SCSI_POLL) && xbf_poll_cmd(xs, desc, 1000)) {
 		DPRINTF("%s: desc %u timed out\n", sc->sc_dev.dv_xname, desc);
 		sc->sc_xs[desc] = NULL;
+		xbf_reclaim_xs(xs, desc);
 		xbf_scsi_done(xs, XS_TIMEOUT);
 		return;
 	}
 }
 
 int
-xbf_submit_cmd(struct scsi_xfer *xs)
+xbf_load_xs(struct scsi_xfer *xs, int desc)
 {
 	struct xbf_softc *sc = xs->sc_link->adapter_softc;
+	struct xbf_sge *sge;
 	union xbf_ring_desc *xrd;
+	bus_dmamap_t map;
+	int i, error, mapflags;
+
+	xrd = &sc->sc_xr->xr_desc[desc];
+	map = sc->sc_xs_map[desc];
+
+	mapflags = (sc->sc_domid << 16);
+	if (ISSET(xs->flags, SCSI_NOSLEEP))
+		mapflags |= BUS_DMA_NOWAIT;
+	else
+		mapflags |= BUS_DMA_WAITOK;
+	if (ISSET(xs->flags, SCSI_DATA_IN))
+		mapflags |= BUS_DMA_READ;
+	else
+		mapflags |= BUS_DMA_WRITE;
+
+	error = bus_dmamap_load(sc->sc_dmat, map, xs->data, xs->datalen,
+	    NULL, mapflags);
+	if (error) {
+		DPRINTF("%s: failed to load %u bytes of data\n",
+		    sc->sc_dev.dv_xname, xs->datalen);
+		return (-1);
+	}
+
+	for (i = 0; i < map->dm_nsegs; i++) {
+		sge = &xrd->xrd_req.req_sgl[i];
+		sge->sge_ref = map->dm_segs[i].ds_addr;
+		sge->sge_first = i > 0 ? 0 :
+		    ((vaddr_t)xs->data & PAGE_MASK) >> XBF_SEC_SHIFT;
+		sge->sge_last = sge->sge_first +
+		    (map->dm_segs[i].ds_len >> XBF_SEC_SHIFT) - 1;
+
+		DPRINTF("%s:   seg %d/%d ref %lu len %lu first %u last %u\n",
+		    sc->sc_dev.dv_xname, i + 1, map->dm_nsegs,
+		    map->dm_segs[i].ds_addr, map->dm_segs[i].ds_len,
+		    sge->sge_first, sge->sge_last);
+
+		KASSERT(sge->sge_last <= 7);
+	}
+
+	xrd->xrd_req.req_nsegs = map->dm_nsegs;
+
+	return (0);
+}
+
+int
+xbf_bounce_xs(struct scsi_xfer *xs, int desc)
+{
+	struct xbf_softc *sc = xs->sc_link->adapter_softc;
 	struct xbf_sge *sge;
+	struct xbf_dma_mem *dma;
+	union xbf_ring_desc *xrd;
+	bus_dmamap_t map;
+	bus_size_t size;
+	int i, error, mapflags;
+
+	xrd = &sc->sc_xr->xr_desc[desc];
+	dma = &sc->sc_xs_bb[desc];
+
+	size = roundup(xs->datalen, PAGE_SIZE);
+	if (size > sc->sc_maxphys)
+		return (EFBIG);
+
+	mapflags = (sc->sc_domid << 16);
+	if (ISSET(xs->flags, SCSI_NOSLEEP))
+		mapflags |= BUS_DMA_NOWAIT;
+	else
+		mapflags |= BUS_DMA_WAITOK;
+
+	error = xbf_dma_alloc(sc, dma, size, size / PAGE_SIZE, mapflags);
+	if (error) {
+		DPRINTF("%s: failed to allocate a %u byte bounce buffer\n",
+		    sc->sc_dev.dv_xname, size);
+		return (error);
+	}
+
+	map = dma->dma_map;
+
+	DPRINTF("%s: bouncing %d bytes via %ld size map with %d segments\n",
+	    sc->sc_dev.dv_xname, xs->datalen, size, map->dm_nsegs);
+
+	if (ISSET(xs->flags, SCSI_DATA_OUT))
+		memcpy((caddr_t)dma->dma_vaddr, xs->data, xs->datalen);
+
+	for (i = 0; i < map->dm_nsegs; i++) {
+		sge = &xrd->xrd_req.req_sgl[i];
+		sge->sge_ref = map->dm_segs[i].ds_addr;
+		sge->sge_first = i > 0 ? 0 :
+		    ((vaddr_t)xs->data & PAGE_MASK) >> XBF_SEC_SHIFT;
+		sge->sge_last = sge->sge_first +
+		    (map->dm_segs[i].ds_len >> XBF_SEC_SHIFT) - 1;
+
+		DPRINTF("%s:   seg %d/%d ref %lu len %lu first %u last %u\n",
+		    sc->sc_dev.dv_xname, i + 1, map->dm_nsegs,
+		    map->dm_segs[i].ds_addr, map->dm_segs[i].ds_len,
+		    sge->sge_first, sge->sge_last);
+
+		KASSERT(sge->sge_last <= 7);
+	}
+
+	xrd->xrd_req.req_nsegs = map->dm_nsegs;
+
+	return (0);
+}
+
+void
+xbf_reclaim_xs(struct scsi_xfer *xs, int desc)
+{
+	struct xbf_softc *sc = xs->sc_link->adapter_softc;
+	struct xbf_dma_mem *dma;
+
+	dma = &sc->sc_xs_bb[desc];
+	if (dma->dma_size == 0)
+		return;
+
+	if (ISSET(xs->flags, SCSI_DATA_IN))
+		memcpy(xs->data, (caddr_t)dma->dma_vaddr, xs->datalen);
+
+	xbf_dma_free(sc, dma);
+}
+
+int
+xbf_submit_cmd(struct scsi_xfer *xs)
+{
+	struct xbf_softc *sc = xs->sc_link->adapter_softc;
+	union xbf_ring_desc *xrd;
 	bus_dmamap_t map;
 	struct scsi_rw *rw;
 	struct scsi_rw_big *rwb;
@@ -419,8 +552,7 @@ xbf_submit_cmd(struct scsi_xfer *xs)
 	uint64_t lba = 0;
 	uint32_t nblk = 0;
 	uint8_t operation = 0;
-	int mapflags;
-	int i, desc, error;
+	int desc, error;
 
 	switch (xs->cmd->opcode) {
 	case READ_BIG:
@@ -471,50 +603,31 @@ xbf_submit_cmd(struct scsi_xfer *xs)
 	xrd = &sc->sc_xr->xr_desc[desc];
 	map = sc->sc_xs_map[desc];
 
-	if (operation == XBF_OP_READ || operation == XBF_OP_WRITE) {
-		mapflags = (sc->sc_domid << 16) | BUS_DMA_NOWAIT;
-		mapflags |= operation == XBF_OP_READ ? BUS_DMA_READ :
-		    BUS_DMA_WRITE;
-		error = bus_dmamap_load(sc->sc_dmat, map, xs->data,
-		    xs->datalen, NULL, mapflags);
-		if (error) {
-			DPRINTF("%s: failed to load %u bytes of data\n",
-			    sc->sc_dev.dv_xname, xs->datalen);
-			return (-1);
-		}
+	xrd->xrd_req.req_op = operation;
+	xrd->xrd_req.req_unit = (uint16_t)sc->sc_unit;
+	xrd->xrd_req.req_sector = lba;
 
-		DPRINTF("%s: desc %u %s%s lba %llu nsec %u segs %u len %u\n",
+	if (operation == XBF_OP_READ || operation == XBF_OP_WRITE) {
+		DPRINTF("%s: desc %u %s%s lba %llu nsec %u len %u\n",
 		    sc->sc_dev.dv_xname, desc, operation == XBF_OP_READ ?
 		    "read" : "write", ISSET(xs->flags, SCSI_POLL) ? "-poll" :
-		    "", lba, nblk, map->dm_nsegs, xs->datalen);
-
-		for (i = 0; i < map->dm_nsegs; i++) {
-			sge = &xrd->xrd_req.req_sgl[i];
-			sge->sge_ref = map->dm_segs[i].ds_addr;
-			sge->sge_first = i > 0 ? 0 :
-			    ((vaddr_t)xs->data & PAGE_MASK) >> XBF_SEC_SHIFT;
-			sge->sge_last = sge->sge_first +
-			    (map->dm_segs[i].ds_len >> XBF_SEC_SHIFT) - 1;
-			DPRINTF("%s:   seg %d ref %lu len %lu first %u "
-			    "last %u\n", sc->sc_dev.dv_xname, i,
-			    map->dm_segs[i].ds_addr, map->dm_segs[i].ds_len,
-			    sge->sge_first, sge->sge_last);
-			KASSERT(sge->sge_last <= 7);
-		}
+		    "", lba, nblk, xs->datalen);
+
+		if (((vaddr_t)xs->data & ((1 << XBF_SEC_SHIFT) - 1)) == 0)
+			error = xbf_load_xs(xs, desc);
+		else
+			error = xbf_bounce_xs(xs, desc);
+		if (error)
+			return (error);
 	} else {
 		DPRINTF("%s: desc %u %s%s lba %llu\n", sc->sc_dev.dv_xname,
 		    desc, operation == XBF_OP_FLUSH ? "flush" : "barrier",
 		    ISSET(xs->flags, SCSI_POLL) ? "-poll" : "", lba);
-		map->dm_nsegs = 0;
+		xrd->xrd_req.req_nsegs = 0;
 	}
 
 	sc->sc_xs[desc] = xs;
 
-	xrd->xrd_req.req_op = operation;
-	xrd->xrd_req.req_nsegs = map->dm_nsegs;
-	xrd->xrd_req.req_unit = (uint16_t)sc->sc_unit;
-	xrd->xrd_req.req_sector = lba;
-
 	bus_dmamap_sync(sc->sc_dmat, map, 0, map->dm_mapsize,
 	    BUS_DMASYNC_PREREAD | BUS_DMASYNC_PREWRITE);
 
@@ -563,7 +676,10 @@ xbf_complete_cmd(struct scsi_xfer *xs, int desc)
 	error = xrd->xrd_rsp.rsp_status == XBF_OK ? XS_NOERROR :
 	    XS_DRIVER_STUFFUP;
 
-	map = sc->sc_xs_map[desc];
+	if (sc->sc_xs_bb[desc].dma_size > 0)
+		map = sc->sc_xs_bb[desc].dma_map;
+	else
+		map = sc->sc_xs_map[desc];
 	bus_dmamap_sync(sc->sc_dmat, map, 0, map->dm_mapsize,
 	    BUS_DMASYNC_POSTREAD | BUS_DMASYNC_POSTWRITE);
 	bus_dmamap_unload(sc->sc_dmat, map);
@@ -579,6 +695,8 @@ xbf_complete_cmd(struct scsi_xfer *xs, int desc)
 	xrd->xrd_req.req_id = id;
 
 	xs->resid = 0;
+
+	xbf_reclaim_xs(xs, desc);
 	xbf_scsi_done(xs, error);
 }
 
@@ -891,13 +1009,21 @@ xbf_init(struct xbf_softc *sc)
 
 int
 xbf_dma_alloc(struct xbf_softc *sc, struct xbf_dma_mem *dma,
-    bus_size_t size, int nseg, int mapflags)
+    bus_size_t size, int nsegs, int mapflags)
 {
 	int error;
 
 	dma->dma_tag = sc->sc_dmat;
 
-	error = bus_dmamap_create(dma->dma_tag, size, nseg, PAGE_SIZE, 0,
+	dma->dma_seg = mallocarray(nsegs, sizeof(bus_dma_segment_t), M_DEVBUF,
+	    M_ZERO | BUS_DMA_NOWAIT);
+	if (dma->dma_seg == NULL) {
+		printf("%s: failed to allocate a segment array\n",
+		    sc->sc_dev.dv_xname);
+		return (ENOMEM);
+	}
+
+	error = bus_dmamap_create(dma->dma_tag, size, nsegs, PAGE_SIZE, 0,
 	    BUS_DMA_NOWAIT, &dma->dma_map);
 	if (error) {
 		printf("%s: failed to create a memory map (%d)\n",
@@ -906,14 +1032,14 @@ xbf_dma_alloc(struct xbf_softc *sc, struct xbf_dma_mem *dma,
 	}
 
 	error = bus_dmamem_alloc(dma->dma_tag, size, PAGE_SIZE, 0,
-	    &dma->dma_seg, nseg, &dma->dma_nsegs, BUS_DMA_NOWAIT);
+	    dma->dma_seg, nsegs, &dma->dma_rsegs, BUS_DMA_NOWAIT);
 	if (error) {
 		printf("%s: failed to allocate DMA memory (%d)\n",
 		    sc->sc_dev.dv_xname, error);
 		goto destroy;
 	}
 
-	error = bus_dmamem_map(dma->dma_tag, &dma->dma_seg, dma->dma_nsegs,
+	error = bus_dmamem_map(dma->dma_tag, dma->dma_seg, dma->dma_rsegs,
 	    size, &dma->dma_vaddr, BUS_DMA_NOWAIT);
 	if (error) {
 		printf("%s: failed to map DMA memory (%d)\n",
@@ -930,15 +1056,17 @@ xbf_dma_alloc(struct xbf_softc *sc, struct xbf_dma_mem *dma,
 	}
 
 	dma->dma_size = size;
+	dma->dma_nsegs = nsegs;
 	return (0);
 
  unmap:
 	bus_dmamem_unmap(dma->dma_tag, dma->dma_vaddr, size);
  free:
-	bus_dmamem_free(dma->dma_tag, &dma->dma_seg, dma->dma_nsegs);
+	bus_dmamem_free(dma->dma_tag, dma->dma_seg, dma->dma_rsegs);
  destroy:
 	bus_dmamap_destroy(dma->dma_tag, dma->dma_map);
  errout:
+	free(dma->dma_seg, M_DEVBUF, nsegs * sizeof(bus_dma_segment_t));
 	dma->dma_map = NULL;
 	dma->dma_tag = NULL;
 	return (error);
@@ -953,9 +1081,12 @@ xbf_dma_free(struct xbf_softc *sc, struct xbf_dma_mem *dma)
 	    BUS_DMASYNC_POSTREAD | BUS_DMASYNC_POSTWRITE);
 	bus_dmamap_unload(dma->dma_tag, dma->dma_map);
 	bus_dmamem_unmap(dma->dma_tag, dma->dma_vaddr, dma->dma_size);
-	bus_dmamem_free(dma->dma_tag, &dma->dma_seg, dma->dma_nsegs);
+	bus_dmamem_free(dma->dma_tag, dma->dma_seg, dma->dma_rsegs);
 	bus_dmamap_destroy(dma->dma_tag, dma->dma_map);
+	free(dma->dma_seg, M_DEVBUF, dma->dma_nsegs * sizeof(bus_dma_segment_t));
+	dma->dma_seg = NULL;
 	dma->dma_map = NULL;
+	dma->dma_size = 0;
 }
 
 int
@@ -990,6 +1121,15 @@ xbf_ring_create(struct xbf_softc *sc)
 	}
 	sc->sc_xs_avail = sc->sc_xr_ndesc;
 
+	/* Bounce buffer maps for unaligned buffers */
+	sc->sc_xs_bb = mallocarray(sc->sc_xr_ndesc, sizeof(struct xbf_dma_mem),
+	    M_DEVBUF, M_ZERO | M_NOWAIT);
+	if (sc->sc_xs_bb == NULL) {
+		printf("%s: failed to allocate bounce buffer maps\n",
+		    sc->sc_dev.dv_xname);
+		goto errout;
+	}
+
 	nsegs = MIN(MAXPHYS / PAGE_SIZE, XBF_MAX_SGE);
 	sc->sc_maxphys = nsegs * PAGE_SIZE;