* [PATCH 2/2] usb: make sure dma buffers are properly allocated
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-29 19:52 ` Denis Orlov
2023-06-30 11:52 ` [PATCH 0/2] usb: small dma allocation fixes Sascha Hauer
2 siblings, 0 replies; 5+ messages in thread
From: Denis Orlov @ 2023-06-29 19:52 UTC (permalink / raw)
To: barebox; +Cc: Denis Orlov
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
^ permalink raw reply [flat|nested] 5+ messages in thread