Intel-GFX Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/i915/gem: Explicitly return error value from eb_relocate helpers
@ 2026-02-04 23:03 Jonathan Cavitt
  2026-02-04 23:56 ` ✗ i915.CI.BAT: failure for " Patchwork
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Jonathan Cavitt @ 2026-02-04 23:03 UTC (permalink / raw)
  To: intel-gfx; +Cc: saurabhg.gupta, alex.zuo, jonathan.cavitt, joonas.lahtinen

Static analysis issue:

The current implementations of the eb_relocate_vma and
eb_relocate_vma_slow functions cast the return value of
eb_relocate_entry to a signed long in order to determine if an error has
occurred.  This is because the return value of eb_relocate_entry is a
u64 offset value on a success and a negative error value on a failure.

While not mechanically incorrect, it is improper to perform a cast like
this.  So, just have eb_relocate_entry (and, by extension, its helper
function relocate_entry) return the error value, storing the offset
separately in a passed u64 pointer.

Interestingly, this value is only used for non-error-checking purposes
in the eb_relocate_vma case.  We can simplify the eb_relocate_vma_slow
case by allowing the passed u64 pointer to be NULL because of this.

Signed-off-by: Jonathan Cavitt <jonathan.cavitt@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
 .../gpu/drm/i915/gem/i915_gem_execbuffer.c    | 43 ++++++++++---------
 1 file changed, 22 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
index d49e96f9be51..450482837604 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
@@ -1373,11 +1373,12 @@ static void clflush_write32(u32 *addr, u32 value, unsigned int flushes)
 	}
 }
 
-static u64
+static int
 relocate_entry(struct i915_vma *vma,
 	       const struct drm_i915_gem_relocation_entry *reloc,
 	       struct i915_execbuffer *eb,
-	       const struct i915_vma *target)
+	       const struct i915_vma *target,
+	       u64 *pin_offset)
 {
 	u64 target_addr = relocation_target(reloc, target);
 	u64 offset = reloc->offset;
@@ -1402,13 +1403,16 @@ relocate_entry(struct i915_vma *vma,
 		goto repeat;
 	}
 
-	return target->node.start | UPDATE;
+	if (pin_offset)
+		*pin_offset = target->node.start | UPDATE;
+	return 0;
 }
 
-static u64
+static int
 eb_relocate_entry(struct i915_execbuffer *eb,
 		  struct eb_vma *ev,
-		  const struct drm_i915_gem_relocation_entry *reloc)
+		  const struct drm_i915_gem_relocation_entry *reloc,
+		  u64 *offset)
 {
 	struct drm_i915_private *i915 = eb->i915;
 	struct eb_vma *target;
@@ -1505,7 +1509,7 @@ eb_relocate_entry(struct i915_execbuffer *eb,
 	ev->flags &= ~EXEC_OBJECT_ASYNC;
 
 	/* and update the user's relocation entry */
-	return relocate_entry(ev->vma, reloc, eb, target->vma);
+	return relocate_entry(ev->vma, reloc, eb, target->vma, offset);
 }
 
 static int eb_relocate_vma(struct i915_execbuffer *eb, struct eb_vma *ev)
@@ -1516,6 +1520,7 @@ static int eb_relocate_vma(struct i915_execbuffer *eb, struct eb_vma *ev)
 	struct drm_i915_gem_relocation_entry __user *urelocs =
 		u64_to_user_ptr(entry->relocs_ptr);
 	unsigned long remain = entry->relocation_count;
+	int err = 0;
 
 	if (unlikely(remain > N_RELOC(INT_MAX)))
 		return -EINVAL;
@@ -1546,21 +1551,21 @@ static int eb_relocate_vma(struct i915_execbuffer *eb, struct eb_vma *ev)
 		copied = __copy_from_user_inatomic(r, urelocs, count * sizeof(r[0]));
 		pagefault_enable();
 		if (unlikely(copied)) {
-			remain = -EFAULT;
+			err = -EFAULT;
 			goto out;
 		}
 
 		remain -= count;
 		do {
-			u64 offset = eb_relocate_entry(eb, ev, r);
+			u64 offset;
+
+			err = eb_relocate_entry(eb, ev, r, &offset);
 
 			if (likely(offset == 0))
 				continue;
 
-			if ((s64)offset < 0) {
-				remain = (int)offset;
+			if (err)
 				goto out;
-			}
 			/*
 			 * Note that reporting an error now
 			 * leaves everything in an inconsistent
@@ -1589,7 +1594,7 @@ static int eb_relocate_vma(struct i915_execbuffer *eb, struct eb_vma *ev)
 	} while (remain);
 out:
 	reloc_cache_reset(&eb->reloc_cache, eb);
-	return remain;
+	return err;
 }
 
 static int
@@ -1599,18 +1604,14 @@ eb_relocate_vma_slow(struct i915_execbuffer *eb, struct eb_vma *ev)
 	struct drm_i915_gem_relocation_entry *relocs =
 		u64_to_ptr(typeof(*relocs), entry->relocs_ptr);
 	unsigned int i;
-	int err;
+	int err = 0;
 
 	for (i = 0; i < entry->relocation_count; i++) {
-		u64 offset = eb_relocate_entry(eb, ev, &relocs[i]);
-
-		if ((s64)offset < 0) {
-			err = (int)offset;
-			goto err;
-		}
+		err = eb_relocate_entry(eb, ev, &relocs[i], NULL);
+		if (err)
+			break;
 	}
-	err = 0;
-err:
+
 	reloc_cache_reset(&eb->reloc_cache, eb);
 	return err;
 }
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread
end of thread, other threads:[~2026-02-24 17:00 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-02-04 23:03 [PATCH] drm/i915/gem: Explicitly return error value from eb_relocate helpers Jonathan Cavitt
2026-02-04 23:56 ` ✗ i915.CI.BAT: failure for " Patchwork
2026-02-21  0:18 ` ✗ i915.CI.BAT: failure for drm/i915/gem: Explicitly return error value from eb_relocate helpers (rev2) Patchwork
2026-02-24 16:19 ` [PATCH] drm/i915/gem: Explicitly return error value from eb_relocate helpers Krzysztof Karas
2026-02-24 16:59   ` Cavitt, Jonathan

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