Re: [PATCH 1/2] drm/i915/gem: Return -EINVAL instead of '0'

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Sun, Jun 16, 2024 at 09:03:48AM GMT, Andi Shyti wrote:
Commit 05da7d9f717b ("drm/i915/gem: Downgrade stolen lmem setup
warning") returns '0' from i915_gem_stolen_lmem_setup(), but it's
supposed to return a pointer to the intel_memory_region
structure.

Sparse complains with the following message:

drivers/gpu/drm/i915/gem/i915_gem_stolen.c:943:32: sparse: sparse:
  Using plain integer as NULL pointer

The caller checks for errors, and if no error is returned, it
stores the address of the stolen memory. Therefore, we can't
return NULL. Since we are handling a case of out-of-bounds, it's
appropriate to treat the "lmem_size < dsm_base" case as an error.

which completely invalidates the point of the commit that introduced this
regression. That was commit was supposed to do "let's continue, just
disabling stolen".  Apparently we should just revert that patch instead
since it introduced 2 new issues and didn't solve what it was supposed
to... for probe failures, we are completely fine leaving the warning
there.


Lucas De Marchi


Return -EINVAL embedded in a pointer instead of '0' (or NULL).

This way, we avoid a potential NULL pointer dereference.

Since we are returning an error, it makes sense to print an error
message with drm_err() instead of a debug message using
drm_dbg().

Fixes: 05da7d9f717b ("drm/i915/gem: Downgrade stolen lmem setup warning")
Signed-off-by: Andi Shyti <andi.shyti@xxxxxxxxxxxxxxx>
Cc: Jonathan Cavitt <jonathan.cavitt@xxxxxxxxx>
---
drivers/gpu/drm/i915/gem/i915_gem_stolen.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_stolen.c b/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
index 004471f60117..bd774ce713cf 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
@@ -937,10 +937,10 @@ i915_gem_stolen_lmem_setup(struct drm_i915_private *i915, u16 type,
		/* Use DSM base address instead for stolen memory */
		dsm_base = intel_uncore_read64(uncore, GEN6_DSMBASE) & GEN11_BDSM_MASK;
		if (lmem_size < dsm_base) {
-			drm_dbg(&i915->drm,
+			drm_err(&i915->drm,
				"Disabling stolen memory support due to OOB placement: lmem_size = %lli vs dsm_base = %lli\n",
				lmem_size, dsm_base);
-			return 0;
+			return ERR_PTR(-EINVAL);
		}
		dsm_size = ALIGN_DOWN(lmem_size - dsm_base, SZ_1M);
	}
--
2.45.1




[Index of Archives]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux