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
