mail archive of the barebox mailing list
 help / color / mirror / Atom feed
From: Denis Orlov <denorl2009@gmail.com>
To: barebox@lists.infradead.org
Cc: Denis Orlov <denorl2009@gmail.com>
Subject: [PATCH 2/2] usb: make sure dma buffers are properly allocated
Date: Thu, 29 Jun 2023 22:52:58 +0300	[thread overview]
Message-ID: <20230629195718.14416-3-denorl2009@gmail.com> (raw)
In-Reply-To: <20230629195718.14416-1-denorl2009@gmail.com>

When we are doing dma_map/unmap we will end up clearing caches. As we
can only do that with the granularity of a cache line, we must ensure
that the corresponding buffers are properly aligned, as otherwise we may
accidentally overwrite some data that happens to reside in the same
cache line. This is exactly what dma_alloc() is for, so use that for
buffers which we are going to map for dma.

Signed-off-by: Denis Orlov <denorl2009@gmail.com>
---
 drivers/usb/core/usb.c          | 16 ++++---
 drivers/usb/storage/transport.c | 74 +++++++++++++++++++--------------
 drivers/usb/storage/usb.c       | 17 ++++----
 3 files changed, 61 insertions(+), 46 deletions(-)

diff --git a/drivers/usb/core/usb.c b/drivers/usb/core/usb.c
index 178ddbbca5..62d9f11146 100644
--- a/drivers/usb/core/usb.c
+++ b/drivers/usb/core/usb.c
@@ -964,15 +964,15 @@ static int usb_string_sub(struct usb_device *dev, unsigned int langid,
  */
 int usb_string(struct usb_device *dev, int index, char *buf, size_t size)
 {
-	unsigned char mybuf[USB_BUFSIZ];
 	unsigned char *tbuf;
-	int err;
+	int err = 0;
 	unsigned int u, idx;
 
 	if (size <= 0 || !buf || !index)
 		return -1;
+
+	tbuf = dma_alloc(USB_BUFSIZ);
 	buf[0] = 0;
-	tbuf = &mybuf[0];
 
 	/* get langid for strings if it's not yet known */
 	if (!dev->have_langid) {
@@ -980,10 +980,12 @@ int usb_string(struct usb_device *dev, int index, char *buf, size_t size)
 		if (err < 0) {
 			dev_dbg(&dev->dev, "error getting string descriptor 0 " \
 				   "(error=%lx)\n", dev->status);
-			return -1;
+			err = -1;
+			goto fail;
 		} else if (tbuf[0] < 4) {
 			pr_debug("string descriptor 0 too short\n");
-			return -1;
+			err = -1;
+			goto fail;
 		} else {
 			dev->have_langid = -1;
 			dev->string_langid = tbuf[2] | (tbuf[3] << 8);
@@ -996,7 +998,7 @@ int usb_string(struct usb_device *dev, int index, char *buf, size_t size)
 
 	err = usb_string_sub(dev, dev->string_langid, index, tbuf);
 	if (err < 0)
-		return err;
+		goto fail;
 
 	size--;		/* leave room for trailing NULL char in output buffer */
 	for (idx = 0, u = 2; u < err; u += 2) {
@@ -1009,6 +1011,8 @@ int usb_string(struct usb_device *dev, int index, char *buf, size_t size)
 	}
 	buf[idx] = 0;
 	err = idx;
+fail:
+	dma_free(tbuf);
 	return err;
 }
 
diff --git a/drivers/usb/storage/transport.c b/drivers/usb/storage/transport.c
index ad347dfce5..be3b18dc66 100644
--- a/drivers/usb/storage/transport.c
+++ b/drivers/usb/storage/transport.c
@@ -86,39 +86,44 @@ int usb_stor_Bulk_transport(struct us_blk_dev *usb_blkdev,
 {
 	struct us_data *us = usb_blkdev->us;
 	struct device *dev = &us->pusb_dev->dev;
-	struct bulk_cb_wrap cbw;
-	struct bulk_cs_wrap csw;
+	struct bulk_cb_wrap *cbw;
+	struct bulk_cs_wrap *csw;
 	int actlen, data_actlen;
 	int result;
 	unsigned int residue;
 	unsigned int pipein = usb_rcvbulkpipe(us->pusb_dev, us->recv_bulk_ep);
 	unsigned int pipeout = usb_sndbulkpipe(us->pusb_dev, us->send_bulk_ep);
 	int dir_in = US_DIRECTION(cmd[0]);
+	int ret = 0;
+
+	cbw = dma_alloc(sizeof(*cbw));
+	csw = dma_alloc(sizeof(*csw));
 
 	/* set up the command wrapper */
-	cbw.Signature = cpu_to_le32(US_BULK_CB_SIGN);
-	cbw.DataTransferLength = cpu_to_le32(datalen);
-	cbw.Flags = (dir_in ? US_BULK_FLAG_IN : US_BULK_FLAG_OUT);
-	cbw.Tag = ++cbw_tag;
-	cbw.Lun = usb_blkdev->lun;
-	cbw.Length = cmdlen;
+	cbw->Signature = cpu_to_le32(US_BULK_CB_SIGN);
+	cbw->DataTransferLength = cpu_to_le32(datalen);
+	cbw->Flags = (dir_in ? US_BULK_FLAG_IN : US_BULK_FLAG_OUT);
+	cbw->Tag = ++cbw_tag;
+	cbw->Lun = usb_blkdev->lun;
+	cbw->Length = cmdlen;
 
 	/* copy the command payload */
-	memset(cbw.CDB, 0, sizeof(cbw.CDB));
-	memcpy(cbw.CDB, cmd, cbw.Length);
+	memset(cbw->CDB, 0, sizeof(cbw->CDB));
+	memcpy(cbw->CDB, cmd, cbw->Length);
 
 	/* send it to out endpoint */
 	dev_dbg(dev, "Bulk Command S 0x%x T 0x%x L %d F %d Trg %d LUN %d CL %d\n",
-		le32_to_cpu(cbw.Signature), cbw.Tag,
-		le32_to_cpu(cbw.DataTransferLength), cbw.Flags,
-		(cbw.Lun >> 4), (cbw.Lun & 0x0F),
-		cbw.Length);
-	result = usb_bulk_msg(us->pusb_dev, pipeout, &cbw, US_BULK_CB_WRAP_LEN,
+		le32_to_cpu(cbw->Signature), cbw->Tag,
+		le32_to_cpu(cbw->DataTransferLength), cbw->Flags,
+		(cbw->Lun >> 4), (cbw->Lun & 0x0F),
+		cbw->Length);
+	result = usb_bulk_msg(us->pusb_dev, pipeout, cbw, US_BULK_CB_WRAP_LEN,
 			      &actlen, USB_BULK_TO);
 	dev_dbg(dev, "Bulk command transfer result=%d\n", result);
 	if (result < 0) {
 		usb_stor_Bulk_reset(us);
-		return USB_STOR_TRANSPORT_FAILED;
+		ret = USB_STOR_TRANSPORT_FAILED;
+		goto fail;
 	}
 
 	/* DATA STAGE */
@@ -141,13 +146,14 @@ int usb_stor_Bulk_transport(struct us_blk_dev *usb_blkdev,
 		if (result < 0) {
 			dev_dbg(dev, "Device status: %lx\n", us->pusb_dev->status);
 			usb_stor_Bulk_reset(us);
-			return USB_STOR_TRANSPORT_FAILED;
+			ret = USB_STOR_TRANSPORT_FAILED;
+			goto fail;
 		}
 	}
 
 	/* STATUS phase + error handling */
 	dev_dbg(dev, "Attempting to get CSW...\n");
-	result = usb_bulk_msg(us->pusb_dev, pipein, &csw, US_BULK_CS_WRAP_LEN,
+	result = usb_bulk_msg(us->pusb_dev, pipein, csw, US_BULK_CS_WRAP_LEN,
 	                      &actlen, USB_BULK_TO);
 
 	/* did the endpoint stall? */
@@ -158,7 +164,7 @@ int usb_stor_Bulk_transport(struct us_blk_dev *usb_blkdev,
 		if (result >= 0) {
 			dev_dbg(dev, "Attempting to get CSW...\n");
 			result = usb_bulk_msg(us->pusb_dev, pipein,
-			                      &csw, US_BULK_CS_WRAP_LEN,
+			                      csw, US_BULK_CS_WRAP_LEN,
 			                      &actlen, USB_BULK_TO);
 		}
 	}
@@ -166,35 +172,39 @@ int usb_stor_Bulk_transport(struct us_blk_dev *usb_blkdev,
 	if (result < 0) {
 		dev_dbg(dev, "Device status: %lx\n", us->pusb_dev->status);
 		usb_stor_Bulk_reset(us);
-		return USB_STOR_TRANSPORT_FAILED;
+		ret = USB_STOR_TRANSPORT_FAILED;
+		goto fail;
 	}
 
 	/* check bulk status */
-	residue = le32_to_cpu(csw.Residue);
+	residue = le32_to_cpu(csw->Residue);
 	dev_dbg(dev, "Bulk Status S 0x%x T 0x%x R %u Stat 0x%x\n",
-		le32_to_cpu(csw.Signature), csw.Tag, residue, csw.Status);
-	if (csw.Signature != cpu_to_le32(US_BULK_CS_SIGN)) {
+		le32_to_cpu(csw->Signature), csw->Tag, residue, csw->Status);
+	if (csw->Signature != cpu_to_le32(US_BULK_CS_SIGN)) {
 		dev_dbg(dev, "Bad CSW signature\n");
 		usb_stor_Bulk_reset(us);
-		return USB_STOR_TRANSPORT_FAILED;
-	} else if (csw.Tag != cbw_tag) {
+		ret = USB_STOR_TRANSPORT_FAILED;
+	} else if (csw->Tag != cbw_tag) {
 		dev_dbg(dev, "Mismatching tag\n");
 		usb_stor_Bulk_reset(us);
-		return USB_STOR_TRANSPORT_FAILED;
-	} else if (csw.Status >= US_BULK_STAT_PHASE) {
+		ret = USB_STOR_TRANSPORT_FAILED;
+	} else if (csw->Status >= US_BULK_STAT_PHASE) {
 		dev_dbg(dev, "Status >= phase\n");
 		usb_stor_Bulk_reset(us);
-		return USB_STOR_TRANSPORT_ERROR;
+		ret = USB_STOR_TRANSPORT_ERROR;
 	} else if (residue > datalen) {
 		dev_dbg(dev, "residue (%uB) > req data (%uB)\n",
 		          residue, datalen);
-		return USB_STOR_TRANSPORT_FAILED;
-	} else if (csw.Status == US_BULK_STAT_FAIL) {
+		ret = USB_STOR_TRANSPORT_FAILED;
+	} else if (csw->Status == US_BULK_STAT_FAIL) {
 		dev_dbg(dev, "FAILED\n");
-		return USB_STOR_TRANSPORT_FAILED;
+		ret = USB_STOR_TRANSPORT_FAILED;
 	}
 
-	return 0;
+fail:
+	dma_free(cbw);
+	dma_free(csw);
+	return ret;
 }
 
 
diff --git a/drivers/usb/storage/usb.c b/drivers/usb/storage/usb.c
index 6cdcc348e4..f281e0186d 100644
--- a/drivers/usb/storage/usb.c
+++ b/drivers/usb/storage/usb.c
@@ -10,6 +10,7 @@
 #include <common.h>
 #include <init.h>
 #include <malloc.h>
+#include <dma.h>
 #include <errno.h>
 #include <scsi.h>
 #include <linux/usb/usb.h>
@@ -33,7 +34,7 @@ static int usb_stor_request_sense(struct us_blk_dev *usb_blkdev)
 	struct device *dev = &us->pusb_dev->dev;
 	u8 cmd[6];
 	const u8 datalen = 18;
-	u8 *data = xzalloc(datalen);
+	u8 *data = dma_alloc(datalen);
 
 	dev_dbg(dev, "SCSI_REQ_SENSE\n");
 
@@ -44,7 +45,7 @@ static int usb_stor_request_sense(struct us_blk_dev *usb_blkdev)
 	dev_dbg(dev, "Request Sense returned %02X %02X %02X\n",
 		data[2], data[12], data[13]);
 
-	free(data);
+	dma_free(data);
 
 	return 0;
 }
@@ -104,7 +105,7 @@ static int usb_stor_inquiry(struct us_blk_dev *usb_blkdev)
 	int ret;
 	u8 cmd[6];
 	const u16 datalen = 36;
-	u8 *data = xzalloc(datalen);
+	u8 *data = dma_alloc(datalen);
 
 	memset(cmd, 0, sizeof(cmd));
 	cmd[0] = SCSI_INQUIRY;
@@ -126,7 +127,7 @@ static int usb_stor_inquiry(struct us_blk_dev *usb_blkdev)
 	// TODO:  process and store device info
 
 exit:
-	free(data);
+	dma_free(data);
 	return ret;
 }
 
@@ -154,7 +155,7 @@ static int read_capacity_16(struct us_blk_dev *usb_blkdev)
 	struct device *dev = &usb_blkdev->us->pusb_dev->dev;
 	unsigned char cmd[16];
 	const u8 datalen = 32;
-	u8 *data = xzalloc(datalen);
+	u8 *data = dma_alloc(datalen);
 	int ret;
 	sector_t lba;
 	unsigned sector_size;
@@ -190,7 +191,7 @@ static int read_capacity_16(struct us_blk_dev *usb_blkdev)
 
 	ret = sector_size;
 fail:
-	free(data);
+	dma_free(data);
 	return ret;
 }
 
@@ -199,7 +200,7 @@ static int read_capacity_10(struct us_blk_dev *usb_blkdev)
 	struct device *dev = &usb_blkdev->us->pusb_dev->dev;
 	unsigned char cmd[16];
 	const u32 datalen = 8;
-	__be32 *data = xzalloc(datalen);
+	__be32 *data = dma_alloc(datalen);
 	int ret;
 	sector_t lba;
 	unsigned sector_size;
@@ -229,7 +230,7 @@ static int read_capacity_10(struct us_blk_dev *usb_blkdev)
 
 	ret = SECTOR_SIZE;
 fail:
-	free(data);
+	dma_free(data);
 	return ret;
 }
 
-- 
2.41.0




  parent reply	other threads:[~2023-06-29 19:59 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-06-29 19:52 [PATCH 0/2] usb: small dma allocation fixes Denis Orlov
2023-06-29 19:52 ` [PATCH 1/2] usb: storage: fix missing calls to free() Denis Orlov
2023-06-30  5:54   ` Ahmad Fatoum
2023-06-29 19:52 ` Denis Orlov [this message]
2023-06-30 11:52 ` [PATCH 0/2] usb: small dma allocation fixes Sascha Hauer

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230629195718.14416-3-denorl2009@gmail.com \
    --to=denorl2009@gmail.com \
    --cc=barebox@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox