On Wed, 6 Sep 2023 14:38:15 +0200
Ketil Johnsen <ketil.johnsen@xxxxxxx> wrote:
> 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().
Yep, I noticed/fixed the problem recently, when working on 32-bit
enablement. Anyway, thanks for the reporting those bugs.
