public inbox for drm-ai-reviews@public-inbox.freedesktop.org
 help / color / mirror / Atom feed
* [PATCH 0/4] Fix checking of iommu_map_sgtable return value
@ 2026-04-21  4:02 Mikko Perttunen
  2026-04-21  4:02 ` [PATCH 1/4] gpu: host1x: Fix iommu_map_sgtable() return value check Mikko Perttunen
                   ` (4 more replies)
  0 siblings, 5 replies; 10+ messages in thread
From: Mikko Perttunen @ 2026-04-21  4:02 UTC (permalink / raw)
  To: Thierry Reding, David Airlie, Simona Vetter, Jonathan Hunter,
	Dmitry Osipenko, Mauro Carvalho Chehab, Rob Clark,
	Dmitry Baryshkov, Abhinav Kumar, Jessica Zhang, Sean Paul,
	Marijn Suijten
  Cc: dri-devel, linux-tegra, linux-kernel, linux-media, linux-arm-msm,
	freedreno, Mikko Perttunen

The return value of iommu_map_sgtable changed from unsigned to signed
quite a while ago, but not all callers were updated. Do that.

Each patch is independent of each other. The drm/msm patch additionally
changes from WARNing in case of mapping error to returning the error
code now available.

Best regards
Mikko

Signed-off-by: Mikko Perttunen <mperttunen@nvidia.com>
---
Mikko Perttunen (4):
      gpu: host1x: Fix iommu_map_sgtable() return value check
      drm/tegra: Fix iommu_map_sgtable() return value check
      drm/msm: Fix iommu_map_sgtable() return value check and avoid WARN
      media: nvidia: tegra-vde: Fix iommu_map_sgtable() return value check

 drivers/gpu/drm/msm/msm_iommu.c                 |  5 +++--
 drivers/gpu/drm/tegra/gem.c                     |  9 ++++++---
 drivers/gpu/host1x/job.c                        | 10 ++++++----
 drivers/media/platform/nvidia/tegra-vde/iommu.c |  9 +++++----
 4 files changed, 20 insertions(+), 13 deletions(-)
---
base-commit: 028ef9c96e96197026887c0f092424679298aae8
change-id: 20260417-iommu_map_sgtable-return-a8d3cd591fbc


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 1/4] gpu: host1x: Fix iommu_map_sgtable() return value check
  2026-04-21  4:02 [PATCH 0/4] Fix checking of iommu_map_sgtable return value Mikko Perttunen
@ 2026-04-21  4:02 ` Mikko Perttunen
  2026-04-22 22:57   ` Claude review: " Claude Code Review Bot
  2026-04-21  4:02 ` [PATCH 2/4] drm/tegra: " Mikko Perttunen
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 10+ messages in thread
From: Mikko Perttunen @ 2026-04-21  4:02 UTC (permalink / raw)
  To: Thierry Reding, David Airlie, Simona Vetter, Jonathan Hunter,
	Dmitry Osipenko, Mauro Carvalho Chehab, Rob Clark,
	Dmitry Baryshkov, Abhinav Kumar, Jessica Zhang, Sean Paul,
	Marijn Suijten
  Cc: dri-devel, linux-tegra, linux-kernel, linux-media, linux-arm-msm,
	freedreno, Mikko Perttunen

Commit "iommu: return full error code from iommu_map_sg[_atomic]()"
changed iommu_map_sgtable() to return an ssize_t and negative values
in error cases, rather than a size_t and a zero.

pin_job() also was incorrectly assigning to 'int', which could cause
overflows into negative values.

Update pin_job() to correctly check for errors from iommu_map_sgtable.

Fixes: ad8f36e4b6b1 ("iommu: return full error code from iommu_map_sg[_atomic]()")
Signed-off-by: Mikko Perttunen <mperttunen@nvidia.com>
---
 drivers/gpu/host1x/job.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/host1x/job.c b/drivers/gpu/host1x/job.c
index 3ed49e1fd933..70bda32f1ff4 100644
--- a/drivers/gpu/host1x/job.c
+++ b/drivers/gpu/host1x/job.c
@@ -235,6 +235,8 @@ static unsigned int pin_job(struct host1x *host, struct host1x_job *job)
 		}
 
 		if (host->domain) {
+			ssize_t map_err;
+
 			for_each_sgtable_sg(map->sgt, sg, j)
 				gather_size += sg->length;
 
@@ -248,11 +250,11 @@ static unsigned int pin_job(struct host1x *host, struct host1x_job *job)
 				goto put;
 			}
 
-			err = iommu_map_sgtable(host->domain, iova_dma_addr(&host->iova, alloc),
-						map->sgt, IOMMU_READ);
-			if (err == 0) {
+			map_err = iommu_map_sgtable(host->domain, iova_dma_addr(&host->iova, alloc),
+						    map->sgt, IOMMU_READ);
+			if (map_err < 0) {
 				__free_iova(&host->iova, alloc);
-				err = -EINVAL;
+				err = map_err;
 				goto put;
 			}
 

-- 
2.53.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 2/4] drm/tegra: Fix iommu_map_sgtable() return value check
  2026-04-21  4:02 [PATCH 0/4] Fix checking of iommu_map_sgtable return value Mikko Perttunen
  2026-04-21  4:02 ` [PATCH 1/4] gpu: host1x: Fix iommu_map_sgtable() return value check Mikko Perttunen
@ 2026-04-21  4:02 ` Mikko Perttunen
  2026-04-22 22:57   ` Claude review: " Claude Code Review Bot
  2026-04-21  4:02 ` [PATCH 3/4] drm/msm: Fix iommu_map_sgtable() return value check and avoid WARN Mikko Perttunen
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 10+ messages in thread
From: Mikko Perttunen @ 2026-04-21  4:02 UTC (permalink / raw)
  To: Thierry Reding, David Airlie, Simona Vetter, Jonathan Hunter,
	Dmitry Osipenko, Mauro Carvalho Chehab, Rob Clark,
	Dmitry Baryshkov, Abhinav Kumar, Jessica Zhang, Sean Paul,
	Marijn Suijten
  Cc: dri-devel, linux-tegra, linux-kernel, linux-media, linux-arm-msm,
	freedreno, Mikko Perttunen

Commit "iommu: return full error code from iommu_map_sg[_atomic]()"
changed iommu_map_sgtable() to return an ssize_t and negative values
in error cases, rather than a size_t and a zero.

Update tegra_bo_iommu_map() to correctly check for errors from
iommu_map_sgtable.

Fixes: ad8f36e4b6b1 ("iommu: return full error code from iommu_map_sg[_atomic]()")
Signed-off-by: Mikko Perttunen <mperttunen@nvidia.com>
---
 drivers/gpu/drm/tegra/gem.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/tegra/gem.c b/drivers/gpu/drm/tegra/gem.c
index d2bae88ad545..684a16be2c0f 100644
--- a/drivers/gpu/drm/tegra/gem.c
+++ b/drivers/gpu/drm/tegra/gem.c
@@ -235,6 +235,7 @@ static const struct host1x_bo_ops tegra_bo_ops = {
 static int tegra_bo_iommu_map(struct tegra_drm *tegra, struct tegra_bo *bo)
 {
 	int prot = IOMMU_READ | IOMMU_WRITE;
+	ssize_t size;
 	int err;
 
 	if (bo->mm)
@@ -256,13 +257,15 @@ static int tegra_bo_iommu_map(struct tegra_drm *tegra, struct tegra_bo *bo)
 
 	bo->iova = bo->mm->start;
 
-	bo->size = iommu_map_sgtable(tegra->domain, bo->iova, bo->sgt, prot);
-	if (!bo->size) {
+	size = iommu_map_sgtable(tegra->domain, bo->iova, bo->sgt, prot);
+	if (size < 0) {
 		dev_err(tegra->drm->dev, "failed to map buffer\n");
-		err = -ENOMEM;
+		err = size;
 		goto remove;
 	}
 
+	bo->size = size;
+
 	mutex_unlock(&tegra->mm_lock);
 
 	return 0;

-- 
2.53.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 3/4] drm/msm: Fix iommu_map_sgtable() return value check and avoid WARN
  2026-04-21  4:02 [PATCH 0/4] Fix checking of iommu_map_sgtable return value Mikko Perttunen
  2026-04-21  4:02 ` [PATCH 1/4] gpu: host1x: Fix iommu_map_sgtable() return value check Mikko Perttunen
  2026-04-21  4:02 ` [PATCH 2/4] drm/tegra: " Mikko Perttunen
@ 2026-04-21  4:02 ` Mikko Perttunen
  2026-04-22 22:57   ` Claude review: " Claude Code Review Bot
  2026-04-21  4:02 ` [PATCH 4/4] media: nvidia: tegra-vde: Fix iommu_map_sgtable() return value check Mikko Perttunen
  2026-04-22 22:57 ` Claude review: Fix checking of iommu_map_sgtable return value Claude Code Review Bot
  4 siblings, 1 reply; 10+ messages in thread
From: Mikko Perttunen @ 2026-04-21  4:02 UTC (permalink / raw)
  To: Thierry Reding, David Airlie, Simona Vetter, Jonathan Hunter,
	Dmitry Osipenko, Mauro Carvalho Chehab, Rob Clark,
	Dmitry Baryshkov, Abhinav Kumar, Jessica Zhang, Sean Paul,
	Marijn Suijten
  Cc: dri-devel, linux-tegra, linux-kernel, linux-media, linux-arm-msm,
	freedreno, Mikko Perttunen

Commit "iommu: return full error code from iommu_map_sg[_atomic]()"
changed iommu_map_sgtable() to return an ssize_t and negative values
in error cases, rather than a size_t and a zero.

Store the return value in the appropriate type and in case of error,
return it rather than WARNing.

Fixes: ad8f36e4b6b1 ("iommu: return full error code from iommu_map_sg[_atomic]()")
Signed-off-by: Mikko Perttunen <mperttunen@nvidia.com>
---
 drivers/gpu/drm/msm/msm_iommu.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/msm/msm_iommu.c b/drivers/gpu/drm/msm/msm_iommu.c
index 7d449e5202c5..058c71c82cf5 100644
--- a/drivers/gpu/drm/msm/msm_iommu.c
+++ b/drivers/gpu/drm/msm/msm_iommu.c
@@ -677,7 +677,7 @@ static int msm_iommu_map(struct msm_mmu *mmu, uint64_t iova,
 			 int prot)
 {
 	struct msm_iommu *iommu = to_msm_iommu(mmu);
-	size_t ret;
+	ssize_t ret;
 
 	WARN_ON(off != 0);
 
@@ -686,7 +686,8 @@ static int msm_iommu_map(struct msm_mmu *mmu, uint64_t iova,
 		iova |= GENMASK_ULL(63, 49);
 
 	ret = iommu_map_sgtable(iommu->domain, iova, sgt, prot);
-	WARN_ON(!ret);
+	if (ret < 0)
+		return ret;
 
 	return (ret == len) ? 0 : -EINVAL;
 }

-- 
2.53.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 4/4] media: nvidia: tegra-vde: Fix iommu_map_sgtable() return value check
  2026-04-21  4:02 [PATCH 0/4] Fix checking of iommu_map_sgtable return value Mikko Perttunen
                   ` (2 preceding siblings ...)
  2026-04-21  4:02 ` [PATCH 3/4] drm/msm: Fix iommu_map_sgtable() return value check and avoid WARN Mikko Perttunen
@ 2026-04-21  4:02 ` Mikko Perttunen
  2026-04-22 22:57   ` Claude review: " Claude Code Review Bot
  2026-04-22 22:57 ` Claude review: Fix checking of iommu_map_sgtable return value Claude Code Review Bot
  4 siblings, 1 reply; 10+ messages in thread
From: Mikko Perttunen @ 2026-04-21  4:02 UTC (permalink / raw)
  To: Thierry Reding, David Airlie, Simona Vetter, Jonathan Hunter,
	Dmitry Osipenko, Mauro Carvalho Chehab, Rob Clark,
	Dmitry Baryshkov, Abhinav Kumar, Jessica Zhang, Sean Paul,
	Marijn Suijten
  Cc: dri-devel, linux-tegra, linux-kernel, linux-media, linux-arm-msm,
	freedreno, Mikko Perttunen

Commit "iommu: return full error code from iommu_map_sg[_atomic]()"
changed iommu_map_sgtable() to return an ssize_t and negative values
in error cases, rather than a size_t and a zero.

Update tegra_vde_iommu_map() to correctly check for errors from
iommu_map_sgtable.

Fixes: ad8f36e4b6b1 ("iommu: return full error code from iommu_map_sg[_atomic]()")
Signed-off-by: Mikko Perttunen <mperttunen@nvidia.com>
---
 drivers/media/platform/nvidia/tegra-vde/iommu.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/media/platform/nvidia/tegra-vde/iommu.c b/drivers/media/platform/nvidia/tegra-vde/iommu.c
index b1d9d841d944..824d2aa80a43 100644
--- a/drivers/media/platform/nvidia/tegra-vde/iommu.c
+++ b/drivers/media/platform/nvidia/tegra-vde/iommu.c
@@ -25,6 +25,7 @@ int tegra_vde_iommu_map(struct tegra_vde *vde,
 	unsigned long shift;
 	unsigned long end;
 	dma_addr_t addr;
+	ssize_t map_err;
 
 	end = vde->domain->geometry.aperture_end;
 	size = iova_align(&vde->iova, size);
@@ -36,11 +37,11 @@ int tegra_vde_iommu_map(struct tegra_vde *vde,
 
 	addr = iova_dma_addr(&vde->iova, iova);
 
-	size = iommu_map_sgtable(vde->domain, addr, sgt,
-				 IOMMU_READ | IOMMU_WRITE);
-	if (!size) {
+	map_err = iommu_map_sgtable(vde->domain, addr, sgt,
+				    IOMMU_READ | IOMMU_WRITE);
+	if (map_err < 0) {
 		__free_iova(&vde->iova, iova);
-		return -ENXIO;
+		return map_err;
 	}
 
 	*iovap = iova;

-- 
2.53.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Claude review: Fix checking of iommu_map_sgtable return value
  2026-04-21  4:02 [PATCH 0/4] Fix checking of iommu_map_sgtable return value Mikko Perttunen
                   ` (3 preceding siblings ...)
  2026-04-21  4:02 ` [PATCH 4/4] media: nvidia: tegra-vde: Fix iommu_map_sgtable() return value check Mikko Perttunen
@ 2026-04-22 22:57 ` Claude Code Review Bot
  4 siblings, 0 replies; 10+ messages in thread
From: Claude Code Review Bot @ 2026-04-22 22:57 UTC (permalink / raw)
  To: dri-devel-reviews

Overall Series Review

Subject: Fix checking of iommu_map_sgtable return value
Author: Mikko Perttunen <mperttunen@nvidia.com>
Patches: 5
Reviewed: 2026-04-23T08:57:23.407241

---

This is a clean, well-motivated bugfix series from Mikko Perttunen. The core issue is straightforward: `iommu_map_sgtable()` changed its return type from `size_t` (returning 0 on error) to `ssize_t` (returning negative errno on error), but several callers were never updated and still check for `== 0` or `!ret`. All four patches correctly update the callers to use `ssize_t` and check `< 0`. Each patch is independent, properly tagged with `Fixes:`, and minimal in scope.

The series looks correct. I have one minor observation on patch 2, but nothing blocking.

**Verdict: Looks good. All four patches are correct and should be applied.**

---

---
Generated by Claude Code Patch Reviewer

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Claude review: gpu: host1x: Fix iommu_map_sgtable() return value check
  2026-04-21  4:02 ` [PATCH 1/4] gpu: host1x: Fix iommu_map_sgtable() return value check Mikko Perttunen
@ 2026-04-22 22:57   ` Claude Code Review Bot
  0 siblings, 0 replies; 10+ messages in thread
From: Claude Code Review Bot @ 2026-04-22 22:57 UTC (permalink / raw)
  To: dri-devel-reviews

Patch Review

**Status: Good**

The old code assigned `iommu_map_sgtable()` directly into `int err`, which has two problems: (1) it checked `== 0` for failure (inverted vs the new API), and (2) assigning `ssize_t` into `int` could truncate large success values into negative numbers, creating false failures. The commit message correctly identifies both issues.

The fix introduces a properly-typed `ssize_t map_err` local, checks `< 0`, and propagates the actual error instead of a hardcoded `-EINVAL`:

```c
+			ssize_t map_err;
...
+			map_err = iommu_map_sgtable(host->domain, iova_dma_addr(&host->iova, alloc),
+						    map->sgt, IOMMU_READ);
+			if (map_err < 0) {
 				__free_iova(&host->iova, alloc);
-				err = -EINVAL;
+				err = map_err;
```

The `err = map_err` assignment narrows `ssize_t` to `int`, but this is fine on the error path since negative errnos always fit in an `int`.

No issues.

---
Generated by Claude Code Patch Reviewer

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Claude review: drm/tegra: Fix iommu_map_sgtable() return value check
  2026-04-21  4:02 ` [PATCH 2/4] drm/tegra: " Mikko Perttunen
@ 2026-04-22 22:57   ` Claude Code Review Bot
  0 siblings, 0 replies; 10+ messages in thread
From: Claude Code Review Bot @ 2026-04-22 22:57 UTC (permalink / raw)
  To: dri-devel-reviews

Patch Review

**Status: Good**

This patch introduces a local `ssize_t size` to capture the return value, checks `< 0`, and only assigns to `bo->size` on success:

```c
+	size = iommu_map_sgtable(tegra->domain, bo->iova, bo->sgt, prot);
+	if (size < 0) {
 		dev_err(tegra->drm->dev, "failed to map buffer\n");
-		err = -ENOMEM;
+		err = size;
 		goto remove;
 	}
+
+	bo->size = size;
```

The deferral of `bo->size = size` to after the error check is a nice improvement -- the old code would write zero into `bo->size` on the error path (since it checked `!bo->size`), which was harmless but sloppy. Now `bo->size` is only set on success.

One minor note: `bo->size` is declared as `size_t` in `gem.h:68`. Assigning from `ssize_t` to `size_t` is safe here because it only happens after the `< 0` check, so the value is guaranteed non-negative. This is fine.

Similarly, `err = size` narrows `ssize_t` to `int` but only on the error path (negative errno), which is safe.

No issues.

---
Generated by Claude Code Patch Reviewer

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Claude review: drm/msm: Fix iommu_map_sgtable() return value check and avoid WARN
  2026-04-21  4:02 ` [PATCH 3/4] drm/msm: Fix iommu_map_sgtable() return value check and avoid WARN Mikko Perttunen
@ 2026-04-22 22:57   ` Claude Code Review Bot
  0 siblings, 0 replies; 10+ messages in thread
From: Claude Code Review Bot @ 2026-04-22 22:57 UTC (permalink / raw)
  To: dri-devel-reviews

Patch Review

**Status: Good**

This patch makes two changes: fixes the type from `size_t` to `ssize_t`, and replaces the `WARN_ON(!ret)` with a proper error return:

```c
-	size_t ret;
+	ssize_t ret;
...
 	ret = iommu_map_sgtable(iommu->domain, iova, sgt, prot);
-	WARN_ON(!ret);
+	if (ret < 0)
+		return ret;
 
 	return (ret == len) ? 0 : -EINVAL;
```

The function `msm_iommu_map` returns `int`, and `ret` is `ssize_t`. On the error path, `return ret` narrows `ssize_t` to `int`, which is safe for negative errnos. On the success path, the existing `(ret == len) ? 0 : -EINVAL` check is preserved, correctly detecting partial mappings.

The removal of `WARN_ON` is an improvement -- a WARN splat in dmesg for a mapping failure that the caller can handle is too noisy. Returning the error code lets the caller decide how to handle it.

No issues.

---
Generated by Claude Code Patch Reviewer

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Claude review: media: nvidia: tegra-vde: Fix iommu_map_sgtable() return value check
  2026-04-21  4:02 ` [PATCH 4/4] media: nvidia: tegra-vde: Fix iommu_map_sgtable() return value check Mikko Perttunen
@ 2026-04-22 22:57   ` Claude Code Review Bot
  0 siblings, 0 replies; 10+ messages in thread
From: Claude Code Review Bot @ 2026-04-22 22:57 UTC (permalink / raw)
  To: dri-devel-reviews

Patch Review

**Status: Good**

Same pattern as patch 1: introduces `ssize_t map_err`, checks `< 0`, propagates the real error instead of hardcoded `-ENXIO`:

```c
+	ssize_t map_err;
...
+	map_err = iommu_map_sgtable(vde->domain, addr, sgt,
+				    IOMMU_READ | IOMMU_WRITE);
+	if (map_err < 0) {
 		__free_iova(&vde->iova, iova);
-		return -ENXIO;
+		return map_err;
```

The function returns `int`, and `return map_err` narrows `ssize_t` to `int` -- safe on the error path.

Note that the old code reused `size` for both the input size parameter and the `iommu_map_sgtable` return. The new code avoids that reuse by using a separate `map_err` variable, which is cleaner since `size` continues to be needed later (it's used as a parameter on line 22 and for iova allocation on line 31).

No issues.

---
Generated by Claude Code Patch Reviewer

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2026-04-22 22:57 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-21  4:02 [PATCH 0/4] Fix checking of iommu_map_sgtable return value Mikko Perttunen
2026-04-21  4:02 ` [PATCH 1/4] gpu: host1x: Fix iommu_map_sgtable() return value check Mikko Perttunen
2026-04-22 22:57   ` Claude review: " Claude Code Review Bot
2026-04-21  4:02 ` [PATCH 2/4] drm/tegra: " Mikko Perttunen
2026-04-22 22:57   ` Claude review: " Claude Code Review Bot
2026-04-21  4:02 ` [PATCH 3/4] drm/msm: Fix iommu_map_sgtable() return value check and avoid WARN Mikko Perttunen
2026-04-22 22:57   ` Claude review: " Claude Code Review Bot
2026-04-21  4:02 ` [PATCH 4/4] media: nvidia: tegra-vde: Fix iommu_map_sgtable() return value check Mikko Perttunen
2026-04-22 22:57   ` Claude review: " Claude Code Review Bot
2026-04-22 22:57 ` Claude review: Fix checking of iommu_map_sgtable return value Claude Code Review Bot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox