Thank you for your contribution! Sashiko AI review found 2 potential issue(s) 
to consider:
- [Medium] drm/panthor: integer truncation of size_t to u32 in 
panthor_kernel_bo_create_from_gem()
- [Low] drm/panthor: unused usage_flags parameter in panthor_gem_create()
--

commit e696a47bb56852d031f4d4fbdb9474523f19a3f1
Author: Boris Brezillon <[email protected]>

drm/panthor: Create kernel BO from existing GEM

Refactors/splits existing panthor_kernel_bo_create(). Adds
panthor_kernel_bo_create_from_gem() so we can create kernel BOs
from imported protected memory in future change.

> diff --git a/drivers/gpu/drm/panthor/panthor_gem.c 
> b/drivers/gpu/drm/panthor/panthor_gem.c
> index 0af4ae47780be..ec6eba8e2ac3a 100644
> --- a/drivers/gpu/drm/panthor/panthor_gem.c
> +++ b/drivers/gpu/drm/panthor/panthor_gem.c
[ ... ]
> @@ -1036,7 +1036,6 @@ panthor_gem_create(struct drm_device *dev, size_t size, 
> uint32_t flags,
>               bo->base.resv = bo->exclusive_vm_root_gem->resv;
>       }
>  
> -     panthor_gem_debugfs_add_usage_flags(bo, usage_flags);
>       panthor_gem_debugfs_bo_add(bo);
>       return bo;

[Severity: Low]
Since the call to panthor_gem_debugfs_add_usage_flags() is removed here, does
this leave the usage_flags parameter entirely unused in panthor_gem_create()? 

Should the parameter be removed from the function signature as well to avoid
dead code?

[ ... ]
> @@ -1274,46 +1275,33 @@ void panthor_kernel_bo_destroy(struct 
> panthor_kernel_bo *bo)
>  }
>  
>  /**
> - * panthor_kernel_bo_create() - Create and map a GEM object to a VM
> + * panthor_kernel_bo_create_from_gem() - Create kernel BO from existing GEM 
> object
[ ... ]
>  struct panthor_kernel_bo *
> -panthor_kernel_bo_create(struct panthor_device *ptdev, struct panthor_vm *vm,
> -                      size_t size, u32 bo_flags, u32 vm_map_flags,
> -                      u64 gpu_va, const char *name)
> +panthor_kernel_bo_create_from_gem(struct panthor_device *ptdev,
> +                               struct panthor_vm *vm,
> +                               struct panthor_gem_object *bo,
> +                               u32 vm_map_flags, u64 gpu_va, u32 vm_map_size)

[Severity: Medium]
Does this cause an implicit integer truncation on 64-bit platforms?

The panthor_kernel_bo_create() function takes size as a size_t (which is 64
bits wide), but passes it to vm_map_size here, which is declared as a u32:

panthor_kernel_bo_create(..., size_t size, ...)
{
        ...
        kbo = panthor_kernel_bo_create_from_gem(ptdev, vm, bo, vm_map_flags,
                                                gpu_va, size);
        ...
}

Could this lead to silent partial allocations or out-of-bounds accesses if a
kernel BO size ever exceeds 4GB?

-- 
Sashiko AI review ยท 
https://sashiko.dev/#/patchset/[email protected]?part=9

Reply via email to