Re: [PATCH 2/2] drm/loongson: Handle buffer mapping failures when clearing a BO

[Icenowy Zheng <zhengxingda@iscas.ac.cn>]

在 2026-10-09五的 16:14 +0200，Christian König写道：
> On 10/9/26 15:50, Icenowy Zheng wrote:
> > 在 2026-10-03六的 09:34 +0000，Evanshenf写道：
> > > lsdc_bo_clear() ignores the return value from lsdc_bo_kmap() and
> > > writes
> > > to lbo->kptr unconditionally. When the first mapping of a newly
> > > allocated
> > > buffer fails, kptr is still NULL and the subsequent memset
> > > accesses
> > > it.
> > > 
> > > Return the mapping error without touching the buffer and
> > > propagate it
> > > from lsdc_gem_object_create(). Install the GEM object functions
> > > before
> > > clearing so that drm_gem_object_put() can release the buffer
> > > correctly
> > > on failure. Do not add the failed object to the tracking list or
> > > return
> > > an uncleared buffer to the caller.
> > > 
> > > Imported buffers still skip clearing, and successfully mapped
> > > buffers
> > > are cleared and unmapped as before.
> > > 
> > > Tested on LS7A2000 with a one-shot range error in ttm_bo_kmap().
> > > Its
> > > -EINVAL return reached userspace, the object was destroyed once,
> > > and
> > > no GEM handle was published. Normal
> > > create/map/zero/write/read/close
> > > cycles passed, with the tracked BO count and VRAM usage
> > > unchanged.
> > > 
> > > AI assistance was used for the lifetime analysis, fix, fault-
> > > injection
> > > tools, build and test execution.
> > > 
> > > Fixes: f39db26c5428 ("drm: Add kms driver for loongson display
> > > controller")
> > > Cc: stable@vger.kernel.org
> > > Assisted-by: LLM
> > > Signed-off-by: Evanshenf <archwse@gmail.com>

I'm going to accept this patch now.

```
Reviewed-by: Icenowy Zheng <zhengxingda@iscas.ac.cn>
```

Although I may prefer to remove the clearing routine in the future.

Thanks,
Icenowy

> > > ---
> > >  drivers/gpu/drm/loongson/lsdc_gem.c | 12 ++++++++----
> > >  drivers/gpu/drm/loongson/lsdc_ttm.c | 10 ++++++++--
> > >  drivers/gpu/drm/loongson/lsdc_ttm.h |  2 +-
> > >  3 files changed, 17 insertions(+), 7 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/loongson/lsdc_gem.c
> > > b/drivers/gpu/drm/loongson/lsdc_gem.c
> > > index 2fb0348..9eba4e1 100644
> > > --- a/drivers/gpu/drm/loongson/lsdc_gem.c
> > > +++ b/drivers/gpu/drm/loongson/lsdc_gem.c
> > > @@ -157,14 +157,18 @@ struct drm_gem_object
> > > *lsdc_gem_object_create(struct drm_device *ddev,
> > >  		return ERR_PTR(ret);
> > >  	}
> > >  
> > > +	gobj = &lbo->tbo.base;
> > > +	gobj->funcs = &lsdc_gem_object_funcs;
> > > +
> > >  	if (!sg) {
> > >  		/* VRAM is filled with random data */
> > > -		lsdc_bo_clear(lbo);
> > > +		ret = lsdc_bo_clear(lbo);
> > > +		if (ret) {
> > > +			drm_gem_object_put(gobj);
> > > +			return ERR_PTR(ret);
> > > +		}
> > >  	}
> > >  
> > > -	gobj = &lbo->tbo.base;
> > > -	gobj->funcs = &lsdc_gem_object_funcs;
> > > -
> > >  	/* tracking the BOs we created */
> > >  	mutex_lock(&ldev->gem.mutex);
> > >  	list_add_tail(&lbo->list, &ldev->gem.objects);
> > > diff --git a/drivers/gpu/drm/loongson/lsdc_ttm.c
> > > b/drivers/gpu/drm/loongson/lsdc_ttm.c
> > > index 88536e2..7b30dea 100644
> > > --- a/drivers/gpu/drm/loongson/lsdc_ttm.c
> > > +++ b/drivers/gpu/drm/loongson/lsdc_ttm.c
> > > @@ -388,9 +388,13 @@ void lsdc_bo_kunmap(struct lsdc_bo *lbo)
> > >  	ttm_bo_kunmap(&lbo->kmap);
> > >  }
> > >  
> > > -void lsdc_bo_clear(struct lsdc_bo *lbo)
> > > +int lsdc_bo_clear(struct lsdc_bo *lbo)
> > >  {
> > > -	lsdc_bo_kmap(lbo);
> > > +	int ret;
> > > +
> > > +	ret = lsdc_bo_kmap(lbo);
> > > +	if (ret)
> > > +		return ret;
> > >  
> > >  	if (lbo->is_iomem)
> > >  		memset_io((void __iomem *)lbo->kptr, 0, lbo-
> > > >size);
> > > @@ -398,6 +402,8 @@ void lsdc_bo_clear(struct lsdc_bo *lbo)
> > >  		memset(lbo->kptr, 0, lbo->size);
> > >  
> > >  	lsdc_bo_kunmap(lbo);
> > > +
> > > +	return 0;
> > >  }
> > >  
> > >  int lsdc_bo_evict_vram(struct drm_device *ddev)
> > 
> > Well I currently don't know whether the clearing is meaningful...
> > 
> > It seems that drm_gem_vram_helper doesn't do this.
> > 
> > Cc'ing TTM maintainers for an answer.
> 
> Natalie and Arun are TTM maintainers now, but I will try to answer
> here as well.
> 
> OpenGL doesn't necessarily require clearing of VRAM nor isolation of
> VRAM data between processes.
> 
> So it is up to the driver if they want this or not. Classically a lot
> of drivers didn't clear VRAM allocations until explicitely asked.
> 
> But if you clear the memory you should have proper error handling and
> a kmap/vmap can fail at any time for multiple reasons.
> 
> Regards,
> Christian.
> 
> > 
> > Thanks,
> > Icenowy
> > 
> > > diff --git a/drivers/gpu/drm/loongson/lsdc_ttm.h
> > > b/drivers/gpu/drm/loongson/lsdc_ttm.h
> > > index 843e147..df47d9b 100644
> > > --- a/drivers/gpu/drm/loongson/lsdc_ttm.h
> > > +++ b/drivers/gpu/drm/loongson/lsdc_ttm.h
> > > @@ -89,7 +89,7 @@ size_t lsdc_bo_size(struct lsdc_bo *lbo);
> > >  
> > >  int lsdc_bo_kmap(struct lsdc_bo *lbo);
> > >  void lsdc_bo_kunmap(struct lsdc_bo *lbo);
> > > -void lsdc_bo_clear(struct lsdc_bo *lbo);
> > > +int lsdc_bo_clear(struct lsdc_bo *lbo);
> > >  
> > >  int lsdc_bo_evict_vram(struct drm_device *ddev);
> > >  
> >