[PATCH v2 12/15] drm/panthor: Add the driver frontend block

[Ketil Johnsen]

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