public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Andrew Davis <afd@ti.com>
To: "Gerd Hoffmann" <kraxel@redhat.com>,
	"Sumit Semwal" <sumit.semwal@linaro.org>,
	"Christian König" <christian.koenig@amd.com>,
	"Paul Cercueil" <paul@crapouillou.net>
Cc: <dri-devel@lists.freedesktop.org>, <linux-media@vger.kernel.org>,
	<linaro-mm-sig@lists.linaro.org>, <linux-kernel@vger.kernel.org>,
	Andrew Davis <afd@ti.com>
Subject: [PATCH 2/3] udmabuf: Sync buffer mappings for attached devices
Date: Tue, 23 Jan 2024 16:12:26 -0600	[thread overview]
Message-ID: <20240123221227.868341-2-afd@ti.com> (raw)
In-Reply-To: <20240123221227.868341-1-afd@ti.com>

Currently this driver creates a SGT table using the CPU as the
target device, then performs the dma_sync operations against
that SGT. This is backwards to how DMA-BUFs are supposed to behave.
This may have worked for the case where these buffers were given
only back to the same CPU that produced them as in the QEMU case.
And only then because the original author had the dma_sync
operations also backwards, syncing for the "device" on begin_cpu.
This was noticed and "fixed" in this patch[0].

That then meant we were sync'ing from the CPU to the CPU using
a pseudo-device "miscdevice". Which then caused another issue
due to the miscdevice not having a proper DMA mask (and why should
it, the CPU is not a DMA device). The fix for that was an even
more egregious hack[1] that declares the CPU is coherent with
itself and can access its own memory space..

Unwind all this and perform the correct action by doing the dma_sync
operations for each device currently attached to the backing buffer.

[0] commit 1ffe09590121 ("udmabuf: fix dma-buf cpu access")
[1] commit 9e9fa6a9198b ("udmabuf: Set the DMA mask for the udmabuf device (v2)")

Signed-off-by: Andrew Davis <afd@ti.com>
---
 drivers/dma-buf/udmabuf.c | 41 +++++++++++++++------------------------
 1 file changed, 16 insertions(+), 25 deletions(-)

diff --git a/drivers/dma-buf/udmabuf.c b/drivers/dma-buf/udmabuf.c
index 3a23f0a7d112a..ab6764322523c 100644
--- a/drivers/dma-buf/udmabuf.c
+++ b/drivers/dma-buf/udmabuf.c
@@ -26,8 +26,6 @@ MODULE_PARM_DESC(size_limit_mb, "Max size of a dmabuf, in megabytes. Default is
 struct udmabuf {
 	pgoff_t pagecount;
 	struct page **pages;
-	struct sg_table *sg;
-	struct miscdevice *device;
 	struct list_head attachments;
 	struct mutex lock;
 };
@@ -169,12 +167,8 @@ static void unmap_udmabuf(struct dma_buf_attachment *at,
 static void release_udmabuf(struct dma_buf *buf)
 {
 	struct udmabuf *ubuf = buf->priv;
-	struct device *dev = ubuf->device->this_device;
 	pgoff_t pg;
 
-	if (ubuf->sg)
-		put_sg_table(dev, ubuf->sg, DMA_BIDIRECTIONAL);
-
 	for (pg = 0; pg < ubuf->pagecount; pg++)
 		put_page(ubuf->pages[pg]);
 	kfree(ubuf->pages);
@@ -185,33 +179,31 @@ static int begin_cpu_udmabuf(struct dma_buf *buf,
 			     enum dma_data_direction direction)
 {
 	struct udmabuf *ubuf = buf->priv;
-	struct device *dev = ubuf->device->this_device;
-	int ret = 0;
-
-	if (!ubuf->sg) {
-		ubuf->sg = get_sg_table(dev, buf, direction);
-		if (IS_ERR(ubuf->sg)) {
-			ret = PTR_ERR(ubuf->sg);
-			ubuf->sg = NULL;
-		}
-	} else {
-		dma_sync_sg_for_cpu(dev, ubuf->sg->sgl, ubuf->sg->nents,
-				    direction);
-	}
+	struct udmabuf_attachment *a;
 
-	return ret;
+	mutex_lock(&ubuf->lock);
+
+	list_for_each_entry(a, &ubuf->attachments, list)
+		dma_sync_sgtable_for_cpu(a->dev, a->table, direction);
+
+	mutex_unlock(&ubuf->lock);
+
+	return 0;
 }
 
 static int end_cpu_udmabuf(struct dma_buf *buf,
 			   enum dma_data_direction direction)
 {
 	struct udmabuf *ubuf = buf->priv;
-	struct device *dev = ubuf->device->this_device;
+	struct udmabuf_attachment *a;
 
-	if (!ubuf->sg)
-		return -EINVAL;
+	mutex_lock(&ubuf->lock);
+
+	list_for_each_entry(a, &ubuf->attachments, list)
+		dma_sync_sgtable_for_device(a->dev, a->table, direction);
+
+	mutex_unlock(&ubuf->lock);
 
-	dma_sync_sg_for_device(dev, ubuf->sg->sgl, ubuf->sg->nents, direction);
 	return 0;
 }
 
@@ -307,7 +299,6 @@ static long udmabuf_create(struct miscdevice *device,
 	exp_info.priv = ubuf;
 	exp_info.flags = O_RDWR;
 
-	ubuf->device = device;
 	buf = dma_buf_export(&exp_info);
 	if (IS_ERR(buf)) {
 		ret = PTR_ERR(buf);
-- 
2.39.2


  reply	other threads:[~2024-01-23 22:12 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-01-23 22:12 [PATCH 1/3] udmabuf: Keep track current device mappings Andrew Davis
2024-01-23 22:12 ` Andrew Davis [this message]
2024-01-24 23:05   ` [PATCH 2/3] udmabuf: Sync buffer mappings for attached devices Kasireddy, Vivek
2024-01-25 15:45     ` Andrew Davis
2024-01-26  7:25       ` Kasireddy, Vivek
2024-01-26 17:40         ` Andrew Davis
2024-01-30  6:23           ` Kasireddy, Vivek
2024-01-25 20:30   ` [Linaro-mm-sig] " Daniel Vetter
2024-01-26 17:24     ` Andrew Davis
2024-01-29 12:27       ` Christian König
2024-01-23 22:12 ` [PATCH 3/3] udmabuf: Use module_misc_device() to register this device Andrew Davis
2024-01-24 22:37   ` Kasireddy, Vivek
2024-01-24 22:36 ` [PATCH 1/3] udmabuf: Keep track current device mappings Kasireddy, Vivek
2024-01-25 15:22   ` Andrew Davis

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=20240123221227.868341-2-afd@ti.com \
    --to=afd@ti.com \
    --cc=christian.koenig@amd.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=kraxel@redhat.com \
    --cc=linaro-mm-sig@lists.linaro.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-media@vger.kernel.org \
    --cc=paul@crapouillou.net \
    --cc=sumit.semwal@linaro.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