Re: [PATCH 2/3] drm/xe/vf: Remove lmtt->ops null check in xe_lmtt_estimate_pt_size

[Rodrigo Vivi <rodrigo.vivi@intel.com>]

On Fri, Mar 08, 2024 at 10:06:50AM +0530, Himal Prasad Ghimiray wrote:
> In xe_lmtt_estimate_pt_size: Pointer is checked against null but then
> dereferenced anyway. 

And what's the problem?

In the line below it access beyond this pointer, so it is a fair
case.

> Since xe_lmtt_init ensures lmtt->ops is populated
> remove the check.

With this in mind we could simply remove all the asserts in the code.

I believe that if someone introduced it here it is likely because
during some development or refactor this ended up being a problem
and want some earlier kind of warning with backtrace information.

> 
> Reported by static analyzer.

Perhaps then replace with an
if (!lmtt->ops) {
    drm_WARN(...);
    return;
}

and/or mark the tool as a false positive?!

> 
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: Himal Prasad Ghimiray <himal.prasad.ghimiray@intel.com>
> ---
>  drivers/gpu/drm/xe/xe_lmtt.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_lmtt.c b/drivers/gpu/drm/xe/xe_lmtt.c
> index 0d7c5514e092..d6d75414bb99 100644
> --- a/drivers/gpu/drm/xe/xe_lmtt.c
> +++ b/drivers/gpu/drm/xe/xe_lmtt.c
> @@ -487,7 +487,6 @@ u64 xe_lmtt_estimate_pt_size(struct xe_lmtt *lmtt, u64 size)
>  
>  	lmtt_assert(lmtt, IS_SRIOV_PF(lmtt_to_xe(lmtt)));
>  	lmtt_assert(lmtt, IS_DGFX(lmtt_to_xe(lmtt)));
> -	lmtt_assert(lmtt, lmtt->ops);
>  
>  	pt_size = PAGE_ALIGN(lmtt->ops->lmtt_pte_size(level) *
>  			     lmtt->ops->lmtt_pte_num(level));
> -- 
> 2.25.1
>