[Intel-xe] [PATCH] drm/xe: Fix overflow in vram manager

[Riana Tauro]

On 3/14/2023 2:22 PM, Matthew Auld wrote:
> On 14/03/2023 08:27, Matthew Auld wrote:
>> On 14/03/2023 04:48, Riana Tauro wrote:
>>> Cc:Matthew Auld <matthew.auld at intel.com>
>>>
>>> On 3/9/2023 10:21 PM, Dixit, Ashutosh wrote:
>>>> On Thu, 09 Mar 2023 05:18:56 -0800, Riana Tauro wrote:
>>>>>
>>>>> The overflow caused xe_bo_restore_kernel to return an error
>>>>> Fix overflow in vram manager alloc function.
>>>>>
>>>>> Signed-off-by: Riana Tauro <riana.tauro at intel.com>
>>>>> ---
>>>>>   drivers/gpu/drm/xe/xe_ttm_vram_mgr.c | 2 +-
>>>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/xe/xe_ttm_vram_mgr.c 
>>>>> b/drivers/gpu/drm/xe/xe_ttm_vram_mgr.c
>>>>> index 643365b18bc7..159ca7105df1 100644
>>>>> --- a/drivers/gpu/drm/xe/xe_ttm_vram_mgr.c
>>>>> +++ b/drivers/gpu/drm/xe/xe_ttm_vram_mgr.c
>>>>> @@ -118,7 +118,7 @@ static int xe_ttm_vram_mgr_new(struct 
>>>>> ttm_resource_manager *man,
>>>>>
>>>>>         cur_size = size;
>>>>>
>>>>> -        if (fpfn + size != place->lpfn << PAGE_SHIFT) {
>>>>> +        if (fpfn + size != (u64)place->lpfn << PAGE_SHIFT) {
>>>>
>>>> Looks ok but not sure if it should just be lpfn computed above? 
>>
>> Yes, that looks correct. place->lpfn should for now only be set when 
>> we want to place something at a specific offset. AFAICT this is 
>> heavily used for pinned kernel objects (as well as setting 
>> ALLOC_CONTIG), and incorrectly applying the roundup_power_of_two() 
>> might break stuff when restoring those objects (the roundup might 
>> smash into other pinned kernel objects).
>>
>> Reviewed-by: Matthew Auld <matthew.auld at intel.com>
> 
> Pushed to drm-xe-next.
> >>
>> fwiw the plan is to replace all of this with the i915 version, which 
>> also fixes this:
>> https://patchwork.freedesktop.org/patch/525697/?series=114521&rev=3
>>
Thank you for the review

Thanks
Riana
>>>> Copying Matt.
>>>>
>>>>>             /*
>>>>>              * Except for actual range allocation, modify the size and
>>>>>              * min_block_size conforming to continuous flag enablement
>>>>> -- 
>>>>> 2.39.1
>>>>>