[nrf fromtree] usb: device_next: msc: do not copy SCSI data

Support only power-of-two disk sector sizes to enable significant
throughput improvements:
  * SCSI data zero-copy
  * allow queuing multi-packet transfers

Previously large SCSI buffers did not improve performance. With this
change, larger SCSI buffer allows scheduling bigger USB transfers which
reduces overhead.

Co-authored-by: Tomasz Moń <tomasz.mon@nordicsemi.no>
Signed-off-by: Tomasz Moń <tomasz.mon@nordicsemi.no>
Signed-off-by: Mark Wang <yichang.wang@nxp.com>
(cherry picked from commit 3c4b4e1)

Author: MarkWangChinese

subsys/usb/device_next/class/usbd_msc.c

@@ -60,11 +60,8 @@ struct CSW {
 /* Single instance is likely enough because it can support multiple LUNs */
 #define MSC_NUM_INSTANCES CONFIG_USBD_MSC_INSTANCES_COUNT
 
-/* Can be 64 if device is not High-Speed capable */
-#define MSC_BUF_SIZE USBD_MAX_BULK_MPS
-
 UDC_BUF_POOL_DEFINE(msc_ep_pool,
-		    MSC_NUM_INSTANCES * 2, MSC_BUF_SIZE,
+		    MSC_NUM_INSTANCES * 2, USBD_MAX_BULK_MPS,
 		    sizeof(struct udc_buf_info), NULL);
 
 struct msc_event {
@@ -121,9 +118,8 @@ struct msc_bot_ctx {
 	struct scsi_ctx luns[CONFIG_USBD_MSC_LUNS_PER_INSTANCE];
 	struct CBW cbw;
 	struct CSW csw;
-	uint8_t scsi_buf[CONFIG_USBD_MSC_SCSI_BUFFER_SIZE];
+	uint8_t *scsi_buf;
 	uint32_t transferred_data;
-	size_t scsi_offset;
 	size_t scsi_bytes;
 };
 
@@ -143,6 +139,53 @@ static struct net_buf *msc_buf_alloc(const uint8_t ep)
 	return buf;
 }
 
+static struct net_buf *msc_buf_alloc_data(const uint8_t ep, uint8_t *data, size_t len)
+{
+	struct net_buf *buf = NULL;
+	struct udc_buf_info *bi;
+
+	buf = net_buf_alloc_with_data(&msc_ep_pool, data, len, K_NO_WAIT);
+	if (buf == NULL) {
+		return NULL;
+	}
+
+	if (USB_EP_DIR_IS_OUT(ep)) {
+		/* Buffer is empty, USB stack will write data from host */
+		buf->len = 0;
+	}
+
+	bi = udc_get_buf_info(buf);
+	bi->ep = ep;
+
+	return buf;
+}
+
+static size_t msc_next_transfer_length(struct usbd_class_data *const c_data)
+{
+	struct usbd_context *uds_ctx = usbd_class_get_ctx(c_data);
+	struct msc_bot_ctx *ctx = usbd_class_get_private(c_data);
+	struct scsi_ctx *lun = &ctx->luns[ctx->cbw.bCBWLUN];
+	size_t len = scsi_cmd_remaining_data_len(lun);
+
+	len = MIN(CONFIG_USBD_MSC_SCSI_BUFFER_SIZE, len);
+
+	/* Limit transfer to bulk endpoint wMaxPacketSize multiple */
+	if (USBD_SUPPORTS_HIGH_SPEED &&
+	    usbd_bus_speed(uds_ctx) == USBD_SPEED_HS) {
+		len = ROUND_DOWN(len, 512);
+	} else {
+		/* Full-Speed */
+		len = ROUND_DOWN(len, 64);
+	}
+
+	/* Round down to sector size multiple */
+	if (lun->sector_size) {
+		len = ROUND_DOWN(len, lun->sector_size);
+	}
+
+	return len;
+}
+
 static uint8_t msc_get_bulk_in(struct usbd_class_data *const c_data)
 {
 	struct usbd_context *uds_ctx = usbd_class_get_ctx(c_data);
@@ -171,10 +214,11 @@ static uint8_t msc_get_bulk_out(struct usbd_class_data *const c_data)
 	return desc->if0_out_ep.bEndpointAddress;
 }
 
-static void msc_queue_bulk_out_ep(struct usbd_class_data *const c_data)
+static void msc_queue_bulk_out_ep(struct usbd_class_data *const c_data, bool data)
 {
 	struct msc_bot_ctx *ctx = usbd_class_get_private(c_data);
 	struct net_buf *buf;
+	uint8_t *scsi_buf = ctx->scsi_buf;
 	uint8_t ep;
 	int ret;
 
@@ -185,7 +229,13 @@ static void msc_queue_bulk_out_ep(struct usbd_class_data *const c_data)
 
 	LOG_DBG("Queuing OUT");
 	ep = msc_get_bulk_out(c_data);
-	buf = msc_buf_alloc(ep);
+
+	if (data) {
+		buf = msc_buf_alloc_data(ep, scsi_buf, msc_next_transfer_length(c_data));
+	} else {
+		buf = msc_buf_alloc(ep);
+	}
+
 	/* The pool is large enough to support all allocations. Failing alloc
 	 * indicates either a memory leak or logic error.
 	 */
@@ -255,50 +305,37 @@ static void msc_process_read(struct msc_bot_ctx *ctx)
 	struct scsi_ctx *lun = &ctx->luns[ctx->cbw.bCBWLUN];
 	int bytes_queued = 0;
 	struct net_buf *buf;
+	uint8_t *scsi_buf = ctx->scsi_buf;
 	uint8_t ep;
-	size_t len;
 	int ret;
 
 	/* Fill SCSI Data IN buffer if there is no data available */
 	if (ctx->scsi_bytes == 0) {
-		ctx->scsi_bytes = scsi_read_data(lun, ctx->scsi_buf);
-		ctx->scsi_offset = 0;
+		size_t len = msc_next_transfer_length(ctx->class_node);
+
+		bytes_queued = scsi_read_data(lun, scsi_buf, len);
+	} else {
+		bytes_queued = ctx->scsi_bytes;
 	}
 
+	/* All data is submitted in one go. Any potential new data will
+	 * have to be retrieved using scsi_read_data() on next call.
+	 */
+	ctx->scsi_bytes = 0;
+
 	if (atomic_test_and_set_bit(&ctx->bits, MSC_BULK_IN_QUEUED)) {
 		__ASSERT_NO_MSG(false);
 		LOG_ERR("IN already queued");
 		return;
 	}
 
 	ep = msc_get_bulk_in(ctx->class_node);
-	buf = msc_buf_alloc(ep);
+	buf = msc_buf_alloc_data(ep, scsi_buf, bytes_queued);
 	/* The pool is large enough to support all allocations. Failing alloc
 	 * indicates either a memory leak or logic error.
 	 */
 	__ASSERT_NO_MSG(buf);
 
-	while (ctx->scsi_bytes - ctx->scsi_offset > 0) {
-		len = MIN(ctx->scsi_bytes - ctx->scsi_offset,
-			  MSC_BUF_SIZE - bytes_queued);
-		if (len == 0) {
-			/* Either queued as much as possible or there is no more
-			 * SCSI IN data available
-			 */
-			break;
-		}
-
-		net_buf_add_mem(buf, &ctx->scsi_buf[ctx->scsi_offset], len);
-		bytes_queued += len;
-		ctx->scsi_offset += len;
-
-		if (ctx->scsi_bytes == ctx->scsi_offset) {
-			/* SCSI buffer can be reused now */
-			ctx->scsi_bytes = scsi_read_data(lun, ctx->scsi_buf);
-			ctx->scsi_offset = 0;
-		}
-	}
-
 	/* Either the net buf is full or there is no more SCSI data */
 	ctx->csw.dCSWDataResidue -= bytes_queued;
 	ret = usbd_ep_enqueue(ctx->class_node, buf);
@@ -319,7 +356,6 @@ static void msc_process_cbw(struct msc_bot_ctx *ctx)
 	cb_len = scsi_usb_boot_cmd_len(ctx->cbw.CBWCB, ctx->cbw.bCBWCBLength);
 	data_len = scsi_cmd(lun, ctx->cbw.CBWCB, cb_len, ctx->scsi_buf);
 	ctx->scsi_bytes = data_len;
-	ctx->scsi_offset = 0;
 	cmd_is_data_read = scsi_cmd_is_data_read(lun);
 	cmd_is_data_write = scsi_cmd_is_data_write(lun);
 	data_len += scsi_cmd_remaining_data_len(lun);
@@ -399,46 +435,17 @@ static void msc_process_write(struct msc_bot_ctx *ctx,
 
 	ctx->transferred_data += len;
 
-	while ((len > 0) && (scsi_cmd_remaining_data_len(lun) > 0)) {
-		/* Copy received data to the end of SCSI buffer */
-		tmp = MIN(len, sizeof(ctx->scsi_buf) - ctx->scsi_bytes);
-		memcpy(&ctx->scsi_buf[ctx->scsi_bytes], buf, tmp);
-		ctx->scsi_bytes += tmp;
-		buf += tmp;
-		len -= tmp;
-
-		/* Pass data to SCSI layer when either all transfer data bytes
-		 * have been received or SCSI buffer is full.
-		 */
-		while ((ctx->scsi_bytes >= scsi_cmd_remaining_data_len(lun)) ||
-		       (ctx->scsi_bytes == sizeof(ctx->scsi_buf))) {
-			tmp = scsi_write_data(lun, ctx->scsi_buf, ctx->scsi_bytes);
-			__ASSERT(tmp <= ctx->scsi_bytes,
-				 "Processed more data than requested");
-			if (tmp == 0) {
-				LOG_WRN("SCSI handler didn't process %d bytes",
-					ctx->scsi_bytes);
-				ctx->scsi_bytes = 0;
-			} else {
-				LOG_DBG("SCSI processed %d out of %d bytes",
-					tmp, ctx->scsi_bytes);
-			}
-
-			ctx->csw.dCSWDataResidue -= tmp;
-			if (scsi_cmd_remaining_data_len(lun) == 0) {
-				/* Abandon any leftover data */
-				ctx->scsi_bytes = 0;
-				break;
-			}
-
-			/* Move remaining data at the start of SCSI buffer. Note
-			 * that the copied length here is zero (and thus no copy
-			 * happens) when underlying sector size is equal to SCSI
-			 * buffer size.
-			 */
-			memmove(ctx->scsi_buf, &ctx->scsi_buf[tmp], ctx->scsi_bytes - tmp);
-			ctx->scsi_bytes -= tmp;
+	if ((len > 0) && (scsi_cmd_remaining_data_len(lun) > 0)) {
+		/* Pass data to SCSI layer. */
+		tmp = scsi_write_data(lun, buf, len);
+		__ASSERT(tmp <= len, "Processed more data than requested");
+		if (tmp == 0) {
+			LOG_WRN("SCSI handler didn't process %d bytes", len);
+		} else {
+			LOG_DBG("SCSI processed %d out of %d bytes", tmp, len);
 		}
+
+		ctx->csw.dCSWDataResidue -= tmp;
 	}
 
 	if ((ctx->transferred_data >= ctx->cbw.dCBWDataTransferLength) ||
@@ -514,7 +521,7 @@ static void msc_handle_bulk_in(struct msc_bot_ctx *ctx,
 		struct scsi_ctx *lun = &ctx->luns[ctx->cbw.bCBWLUN];
 
 		ctx->transferred_data += len;
-		if (ctx->scsi_bytes == 0) {
+		if (msc_next_transfer_length(ctx->class_node) == 0) {
 			if (ctx->csw.dCSWDataResidue > 0) {
 				/* Case (5) Hi > Di
 				 * While we may have sent short packet, device
@@ -623,9 +630,11 @@ static void usbd_msc_thread(void *arg1, void *arg2, void *arg3)
 
 		switch (ctx->state) {
 		case MSC_BBB_EXPECT_CBW:
+			msc_queue_bulk_out_ep(evt.c_data, false);
+			break;
 		case MSC_BBB_PROCESS_WRITE:
 			/* Ensure we can accept next OUT packet */
-			msc_queue_bulk_out_ep(evt.c_data);
+			msc_queue_bulk_out_ep(evt.c_data, true);
 			break;
 		default:
 			break;
@@ -645,7 +654,7 @@ static void usbd_msc_thread(void *arg1, void *arg2, void *arg3)
 		if (ctx->state == MSC_BBB_PROCESS_READ) {
 			msc_process_read(ctx);
 		} else if (ctx->state == MSC_BBB_PROCESS_WRITE) {
-			msc_queue_bulk_out_ep(evt.c_data);
+			msc_queue_bulk_out_ep(evt.c_data, true);
 		} else if (ctx->state == MSC_BBB_SEND_CSW) {
 			msc_send_csw(ctx);
 		}
@@ -864,14 +873,17 @@ struct usbd_class_api msc_bot_api = {
 	.init = msc_bot_init,
 };
 
-#define DEFINE_MSC_BOT_CLASS_DATA(x, _)					\
-	static struct msc_bot_ctx msc_bot_ctx_##x = {			\
-		.desc = &msc_bot_desc_##x,				\
-		.fs_desc = msc_bot_fs_desc_##x,				\
-		.hs_desc = msc_bot_hs_desc_##x,				\
-	};								\
-									\
-	USBD_DEFINE_CLASS(msc_##x, &msc_bot_api, &msc_bot_ctx_##x,	\
+#define DEFINE_MSC_BOT_CLASS_DATA(x, _)						\
+	UDC_STATIC_BUF_DEFINE(scsi_buf_##x, CONFIG_USBD_MSC_SCSI_BUFFER_SIZE);	\
+										\
+	static struct msc_bot_ctx msc_bot_ctx_##x = {				\
+		.desc = &msc_bot_desc_##x,					\
+		.fs_desc = msc_bot_fs_desc_##x,					\
+		.hs_desc = msc_bot_hs_desc_##x,					\
+		.scsi_buf = scsi_buf_##x					\
+	};									\
+										\
+	USBD_DEFINE_CLASS(msc_##x, &msc_bot_api, &msc_bot_ctx_##x,		\
 			  &msc_bot_vregs);
 
 LISTIFY(MSC_NUM_INSTANCES, DEFINE_MSC_BOT_DESCRIPTOR, ())

subsys/usb/device_next/class/usbd_msc_scsi.c

@@ -7,6 +7,7 @@
 #include <zephyr/kernel.h>
 #include <zephyr/storage/disk_access.h>
 #include <zephyr/sys/byteorder.h>
+#include <zephyr/usb/usbd.h>
 
 #include "usbd_msc_scsi.h"
 
@@ -339,6 +340,24 @@ static int update_disk_info(struct scsi_ctx *const ctx)
 		status = -EIO;
 	}
 
+	if (!ctx->sector_size) {
+		status = -EIO;
+	} else if ((ctx->sector_size % USBD_MAX_BULK_MPS) &&
+		   (USBD_MAX_BULK_MPS % ctx->sector_size)) {
+		/* Zephyr MSC class implementation initially allowed any sector
+		 * size, however doing so requires bouncing which significantly
+		 * impedes throughput. To enable zero-copy and scheduling larger
+		 * transfers, the implementation is now restricted to work only
+		 * with power of two disk sizes.
+		 *
+		 * USB bulk wMaxPacketSize is 64 (Full-Speed), 512 (High-Speed)
+		 * or 1024 (Super-Speed) and most common disk sector sizes are
+		 * either 512 or 4096. Therefore the power of two limitation
+		 * shouldn't have effect on any actual application.
+		 */
+		status = -EIO;
+	}
+
 	if (ctx->sector_size > CONFIG_USBD_MSC_SCSI_BUFFER_SIZE) {
 		status = -ENOMEM;
 	}
@@ -707,12 +726,11 @@ validate_transfer_length(struct scsi_ctx *ctx, uint32_t lba, uint16_t length)
 	return 0;
 }
 
-static size_t fill_read_10(struct scsi_ctx *ctx,
-			   uint8_t buf[static CONFIG_USBD_MSC_SCSI_BUFFER_SIZE])
+static size_t fill_read_10(struct scsi_ctx *ctx, uint8_t *buf, size_t length)
 {
 	uint32_t sectors;
 
-	sectors = MIN(CONFIG_USBD_MSC_SCSI_BUFFER_SIZE, ctx->remaining_data) / ctx->sector_size;
+	sectors = MIN(length, ctx->remaining_data) / ctx->sector_size;
 	if (disk_access_read(ctx->disk, buf, ctx->lba, sectors) != 0) {
 		/* Terminate transfer */
 		sectors = 0;
@@ -897,15 +915,14 @@ size_t scsi_cmd_remaining_data_len(struct scsi_ctx *ctx)
 	return ctx->remaining_data;
 }
 
-size_t scsi_read_data(struct scsi_ctx *ctx,
-		      uint8_t buf[static CONFIG_USBD_MSC_SCSI_BUFFER_SIZE])
+size_t scsi_read_data(struct scsi_ctx *ctx, uint8_t *buf, size_t length)
 {
 	size_t retrieved = 0;
 
 	__ASSERT_NO_MSG(ctx->cmd_is_data_read);
 
 	if ((ctx->remaining_data > 0) && ctx->read_cb) {
-		retrieved = ctx->read_cb(ctx, buf);
+		retrieved = ctx->read_cb(ctx, buf, length);
 	}
 	ctx->remaining_data -= retrieved;
 	if (retrieved == 0) {

subsys/usb/device_next/class/usbd_msc_scsi.h

@@ -66,8 +66,7 @@ struct scsi_ctx {
 	const char *vendor;
 	const char *product;
 	const char *revision;
-	size_t (*read_cb)(struct scsi_ctx *ctx,
-			  uint8_t buf[static CONFIG_USBD_MSC_SCSI_BUFFER_SIZE]);
+	size_t (*read_cb)(struct scsi_ctx *ctx, uint8_t *buf, size_t length);
 	size_t (*write_cb)(struct scsi_ctx *ctx, const uint8_t *buf, size_t length);
 	size_t remaining_data;
 	uint32_t lba;
@@ -92,8 +91,7 @@ size_t scsi_cmd(struct scsi_ctx *ctx, const uint8_t *cb, int len,
 bool scsi_cmd_is_data_read(struct scsi_ctx *ctx);
 bool scsi_cmd_is_data_write(struct scsi_ctx *ctx);
 size_t scsi_cmd_remaining_data_len(struct scsi_ctx *ctx);
-size_t scsi_read_data(struct scsi_ctx *ctx,
-		      uint8_t data_in_buf[static CONFIG_USBD_MSC_SCSI_BUFFER_SIZE]);
+size_t scsi_read_data(struct scsi_ctx *ctx, uint8_t *data_in_buf, size_t length);
 size_t scsi_write_data(struct scsi_ctx *ctx, const uint8_t *buf, size_t length);
 
 enum scsi_status_code scsi_cmd_get_status(struct scsi_ctx *ctx);