Re: [PATCH v3 01/36] drm/xe: Add NULL checks to scratch LRC allocation

[Matthew Brost <matthew.brost@intel.com>]

On Tue, Sep 30, 2025 at 04:06:55AM +0200, Lis, Tomasz wrote:
> 
> On 9/29/2025 4:55 AM, Matthew Brost wrote:
> > kmalloc can fail, the returned value must have a NULL check.
> > 
> > Fixes: 168b5867318b ("drm/xe/vf: Refresh utilization buffer during migration recovery")
> > Signed-off-by: Matthew Brost<matthew.brost@intel.com>
> > ---
> >   drivers/gpu/drm/xe/xe_lrc.c | 10 ++++++++--
> >   1 file changed, 8 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/xe/xe_lrc.c b/drivers/gpu/drm/xe/xe_lrc.c
> > index 47e9df775072..e1bc102a6cae 100644
> > --- a/drivers/gpu/drm/xe/xe_lrc.c
> > +++ b/drivers/gpu/drm/xe/xe_lrc.c
> > @@ -1303,8 +1303,11 @@ static int setup_wa_bb(struct xe_lrc *lrc, struct xe_hw_engine *hwe)
> >   	u32 *buf = NULL;
> >   	int ret;
> > -	if (lrc->bo->vmap.is_iomem)
> > +	if (lrc->bo->vmap.is_iomem) {
> >   		buf = kmalloc(LRC_WA_BB_SIZE, GFP_KERNEL);
> > +		if (!buf)
> > +			return -ENOMEM;
> > +	}
> >   	ret = xe_lrc_setup_wa_bb_with_scratch(lrc, hwe, buf);
> 
> xe_lrc_setup_wa_bb_with_scratch()->setup_bo() handles the ENOMEM return, there was no bug.
> 
> > @@ -1347,8 +1350,11 @@ setup_indirect_ctx(struct xe_lrc *lrc, struct xe_hw_engine *hwe)
> >   	if (xe_gt_WARN_ON(lrc->gt, !state.funcs))
> >   		return 0;
> > -	if (lrc->bo->vmap.is_iomem)
> > +	if (lrc->bo->vmap.is_iomem) {
> >   		state.buffer = kmalloc(state.max_size, GFP_KERNEL);
> > +		if (!state.buffer)
> > +			return -ENOMEM;
> > +	}
> >   	ret = setup_bo(&state);
> 
> setup_bo() does another check with ENOMEM return, no bug here as well. Also,
> with how setup_bo() exits, it ignores lack of allocation in case it won't be
> used anyway. -Tomasz
> 

Ok, I see this now but stylically / from layering this simply not how it
is done in Linux - the pattern is basically always kmallc + immediate
NUL check.

I can drop the fixes tag as it don't require a backport to stable but
for style and reability we should merge this patch or a version of this.

Matt

> >   	if (ret) {