Quoting Matthew Auld (2020-01-17 11:51:53)
> On Thu, 16 Jan 2020 at 21:19, Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> wrote:
> >
> > Quoting Matthew Auld (2020-01-16 20:31:49)
> > > Don't allow a mismatch between obj->base.size/vma->size and the actual
> > > number of pages for the backing store, which is limited to INT_MAX
> > > pages.
> > >
> > > Signed-off-by: Matthew Auld <matthew.auld@xxxxxxxxx>
> > > Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> > > ---
> > > drivers/gpu/drm/i915/gem/i915_gem_userptr.c | 12 ++++++++++++
> > > 1 file changed, 12 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_userptr.c b/drivers/gpu/drm/i915/gem/i915_gem_userptr.c
> > > index e5558af111e2..fef96a303d9d 100644
> > > --- a/drivers/gpu/drm/i915/gem/i915_gem_userptr.c
> > > +++ b/drivers/gpu/drm/i915/gem/i915_gem_userptr.c
> > > @@ -768,6 +768,18 @@ i915_gem_userptr_ioctl(struct drm_device *dev,
> > > if (args->flags & ~(I915_USERPTR_READ_ONLY |
> > > I915_USERPTR_UNSYNCHRONIZED))
> > > return -EINVAL;
> > > + /*
> > > + * XXX: There is a prevalence of the assumption that we fit the
> > > + * object's page count inside a 32bit _signed_ variable. Let's document
> > > + * this and catch if we ever need to fix it. In the meantime, if you do
> > > + * spot such a local variable, please consider fixing!
> > > + */
> > > +
> > > + if (args->user_size >> PAGE_SHIFT > INT_MAX)
> > > + return -E2BIG;
> >
> > I'm convinced that the following patch is the last bug (excusing
> > i915_gem_internal.c), and think we should commit to removing this limit.
>
> You mean on our side? There is still all the sg_table stuff,
> __get_user_pages_fast etc.
Didn't notice the get_user_pages -- some use long, sone ints. oops.
sg_table I was thinking of just the sg_length snafu that we work around.
We can kill off sg_table itself as we never pass that outside of the
driver, and just assume our chunking is correct. (Basically lifting more
of lib/scatterlist.c into our control, one day we really should tell
them their code doesn't scale to our use.)
Ok. Let's collate this information into something like
/*
* XXX: There is a prevalence of the assumption that we fit the
* object's page count inside a 32bit _signed_ variable. Let's document
* this and catch if we ever need to fix it. In the meantime, if you do
* spot such a local variable, please consider fixing!
*
* Aside from our own locals (for which we have no excuse!):
* - sg_table embeds unsigned int for num_pages
* - get_user_pages*() mixed ints with longs
*/
We can send patches for get_user_pages...
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
