Re: [PATCH 3/3] drm/xe: Change return type of detect_bar2_dgfx() from s64 to u64

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

On Thu, Oct 09, 2025 at 02:06:40PM -0400, Lin, Shuicheng wrote:
> On Thu, Oct 9, 2025 7:53 AM Rodrigo Vivi wrote:
> > On Mon, Oct 06, 2025 at 08:13:27PM +0000, Shuicheng Lin wrote:
> > > The function never returns a negative value, and the return value is
> > > assigned to a u64 variable. Use u64 for better type correctness and
> > > clarity.
> > >
> > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > Signed-off-by: Shuicheng Lin <shuicheng.lin@intel.com>
> > > ---
> > >  drivers/gpu/drm/xe/xe_ttm_stolen_mgr.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/gpu/drm/xe/xe_ttm_stolen_mgr.c
> > > b/drivers/gpu/drm/xe/xe_ttm_stolen_mgr.c
> > > index 9b00bdc8ef1a..d8ded2400706 100644
> > > --- a/drivers/gpu/drm/xe/xe_ttm_stolen_mgr.c
> > > +++ b/drivers/gpu/drm/xe/xe_ttm_stolen_mgr.c
> > > @@ -81,7 +81,7 @@ static u32 get_wopcm_size(struct xe_device *xe)
> > >  	return wopcm_size;
> > >  }
> > >
> > > -static s64 detect_bar2_dgfx(struct xe_device *xe, struct
> > > xe_ttm_stolen_mgr *mgr)
> > > +static u64 detect_bar2_dgfx(struct xe_device *xe, struct
> > > +xe_ttm_stolen_mgr *mgr)
> > 
> > hmmm.... because stolen size is u64, this is right.
> > 
> > But looking to this function below, I'm wondering...
> > 
> > Although theoritical case, perhaps we should convert stolen_size to s64 and
> > then fail if stolen_size -= wopcm_size took it to the negative side?
> > 
> > Lucas, Thomas, thoughts?
> 
> How about add an assertion like below?
> +       xe_assert(xe, stolen_size > wopcm_size);

I like this assert

>         stolen_size -= wopcm_size;
> 
> Or add drm_WARN_ON that already used in this function.
> +       if (drm_WARN_ON(&xe->drm, stolen_size < stolen_size))
> +               return 0;
> 
> xe_assert() is controlled by CONFIG_DRM_XE_DEBUG. 
> 
> Shuicheng
> 
> > 
> > >  {
> > >  	struct xe_vram_region *tile_vram = xe_device_get_root_tile(xe)-
> > >mem.vram;
> > >  	resource_size_t tile_io_start = xe_vram_region_io_start(tile_vram);
> > > --
> > > 2.49.0
> > >