RE: [bug report] drm/i915/gem: Downgrade stolen lmem setup warning

["Cavitt, Jonathan" <jonathan.cavitt@xxxxxxxxx>]

Just had an internal meeting.  It turns out I mistook this new issue for an older one.
My apologies for the confusion.

A fix for this issue was submitted: https://patchwork.freedesktop.org/series/136053/

Thank you for your time.

-Jonathan Cavitt

-----Original Message-----
From: Cavitt, Jonathan <jonathan.cavitt@xxxxxxxxx> 
Sent: Friday, July 12, 2024 12:17 PM
To: Dan Carpenter <dan.carpenter@xxxxxxxxxx>
Cc: intel-gfx@xxxxxxxxxxxxxxxxxxxxx; Chris Wilson <chris.p.wilson@xxxxxxxxxxxxxxx>; Shyti, Andi <andi.shyti@xxxxxxxxx>; Cavitt, Jonathan <jonathan.cavitt@xxxxxxxxx>
Subject: RE: [bug report] drm/i915/gem: Downgrade stolen lmem setup warning

Hello.  This issue should be resolved by the commit "drm/i915/gem: Return NULL instead of '0'".

I thought this had landed already, but I can't find it in the repo currently.

I'd recommend asking Andi or Chris about this.  I'll CC them about this.

Thank you for your patience.
-Jonathan Cavitt

-----Original Message-----
From: Dan Carpenter <dan.carpenter@xxxxxxxxxx> 
Sent: Friday, July 12, 2024 7:04 AM
To: Cavitt, Jonathan <jonathan.cavitt@xxxxxxxxx>
Cc: intel-gfx@xxxxxxxxxxxxxxxxxxxxx
Subject: [bug report] drm/i915/gem: Downgrade stolen lmem setup warning

Hello Jonathan Cavitt,

Commit 05da7d9f717b ("drm/i915/gem: Downgrade stolen lmem setup
warning") from Apr 22, 2024 (linux-next), leads to the following
Smatch static checker warning:

	drivers/gpu/drm/i915/intel_memory_region.c:371 intel_memory_regions_hw_probe()
	error: potential NULL/IS_ERR bug 'mem'

drivers/gpu/drm/i915/intel_memory_region.c
    327 int intel_memory_regions_hw_probe(struct drm_i915_private *i915)
    328 {
    329         int err, i;
    330 
    331         for (i = 0; i < ARRAY_SIZE(i915->mm.regions); i++) {
    332                 struct intel_memory_region *mem = ERR_PTR(-ENODEV);
    333                 u16 type, instance;
    334 
    335                 if (!HAS_REGION(i915, i))
    336                         continue;
    337 
    338                 type = intel_region_map[i].class;
    339                 instance = intel_region_map[i].instance;
    340                 switch (type) {
    341                 case INTEL_MEMORY_SYSTEM:
    342                         if (IS_DGFX(i915))
    343                                 mem = i915_gem_ttm_system_setup(i915, type,
    344                                                                 instance);
    345                         else
    346                                 mem = i915_gem_shmem_setup(i915, type,
    347                                                            instance);
    348                         break;
    349                 case INTEL_MEMORY_STOLEN_LOCAL:
    350                         mem = i915_gem_stolen_lmem_setup(i915, type, instance);


i915_gem_stolen_lmem_setup() used to only return error pointers but now it
returns NULL as well.

    351                         if (!IS_ERR(mem))
    352                                 i915->mm.stolen_region = mem;
    353                         break;
    354                 case INTEL_MEMORY_STOLEN_SYSTEM:
    355                         mem = i915_gem_stolen_smem_setup(i915, type, instance);
    356                         if (!IS_ERR(mem))
    357                                 i915->mm.stolen_region = mem;
    358                         break;
    359                 default:
    360                         continue;
    361                 }
    362 
    363                 if (IS_ERR(mem)) {
    364                         err = PTR_ERR(mem);
    365                         drm_err(&i915->drm,
    366                                 "Failed to setup region(%d) type=%d\n",
    367                                 err, type);
    368                         goto out_cleanup;
    369                 }
    370 
--> 371                 mem->id = i;
                        ^^^^^^^^^^^^
Potentially leading to a NULL dereference here?

    372                 i915->mm.regions[i] = mem;
    373         }
    374 
    375         for (i = 0; i < ARRAY_SIZE(i915->mm.regions); i++) {
    376                 struct intel_memory_region *mem = i915->mm.regions[i];
    377                 u64 region_size, io_size;

regards,
dan carpenter