On 8/9/23 18:53, Boris Brezillon wrote:
+static int panthor_ioctl_vm_create(struct drm_device *ddev, void *data,
+                                  struct drm_file *file)
+{
+       struct panthor_device *ptdev = container_of(ddev, struct 
panthor_device, base);
+       u32 va_bits = GPU_MMU_FEATURES_VA_BITS(ptdev->gpu_info.mmu_features);
+       struct panthor_file *pfile = file->driver_priv;
+       struct drm_panthor_vm_create *args = data;
+       u64 kernel_va_start = 0;
+       int cookie, ret;
+
+       if (!drm_dev_enter(ddev, &cookie))
+               return -ENODEV;
+
+       if (args->flags & ~PANTHOR_VM_CREATE_FLAGS) {
+               ret = -EINVAL;
+               goto out_dev_exit;
+       }
+
+       if (drm_WARN_ON(ddev, !va_bits) || args->kernel_va_range > (1ull << 
(va_bits - 1))) {
+               ret = -EINVAL;
+               goto out_dev_exit;
+       }
+
+       if (args->kernel_va_range)
+               kernel_va_start = (1 << (va_bits - 1)) - args->kernel_va_range;

Bug here if user space provides kernel_va_range, which is the intention of the current Mesa proposal.

I think the desired calculation should be something like:
kernel_va_start = (1ull << va_bits) - args->kernel_va_range;

PS: There is currently also a bug in the accompanying Mesa changes which accidentally makes kernel_va_range always zero, thus bypassing this kernel bug. The Mesa bug is due to va_bits always being zero because mmu_features field is not copied in panthor_dev_query_props().

+       ret = panthor_vm_pool_create_vm(ptdev, pfile->vms,
+                                       kernel_va_start, args->kernel_va_range);
+       if (ret >= 0) {
+               args->id = ret;
+               ret = 0;
+       }
+
+out_dev_exit:
+       drm_dev_exit(cookie);
+       return ret;
+}

--
Regards,
Ketil Johnsen

Reply via email to