[Intel-gfx] [igt-dev] [PATCH i-g-t v5 1/4] lib/i915: Add minimum GTT alignment helper

[Chris Wilson]

Quoting Joonas Lahtinen (2019-11-05 09:14:20)
> Quoting Janusz Krzysztofik (2019-11-04 19:13:27)
> > Some tests assume 4kB offset alignment while using softpin.  That
> > assumption may be wrong on future GEM backends with possibly larger
> > minimum page sizes.  As a result, those tests may either fail on
> > softpin at offsets which are incorrectly aligned, may silently skip
> > such incorrectly aligned addresses assuming them occupied by other
> > users, or may always succeed when examining invalid use patterns.
> > 
> > Provide a helper function that detects minimum GTT alignment.  Tests
> > may use it to calculate softpin offsets valid on actually used backing
> > store.
> > 
> > Also expose a new object validation helper just created, it may be
> > useful for checking if a shared GTT address is not reserved, for
> > example.
> > 
> > v2: Rename the helper, use 'minimum GTT alignment' term across the
> >     change (Chris),
> >   - use error numbers to distinguish between invalid offsets and
> >     addresses occupied by other users,
> >   - simplify the code (Chris).
> > v3: Put the code under lib/i915/, not in lib/ioctl_wrappers.c (Chris),
> >   - validate objects with an invalid reloc applied so execbuf requests
> >     called only for validation purposes are actually not emitted to GPU
> >     (Chris),
> >   - move object validation code to a separate helper.
> > 
> > Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik at linux.intel.com>
> > Cc: Chris Wilson <chris at chris-wilson.co.uk>
> > Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio at intel.com>
> > Cc: Stuart Summers <stuart.summers at intel.com>
> 
> <SNIP>
> 
> > +int gem_gtt_validate_object(int fd, struct drm_i915_gem_exec_object2 *obj)
> > +{
> > +       struct drm_i915_gem_relocation_entry reloc;
> > +       struct drm_i915_gem_execbuffer2 execbuf;
> > +       const uint32_t bbe = MI_BATCH_BUFFER_END;
> > +       uintptr_t old_reloc;
> > +       int old_count, err;
> > +
> > +       memset(&reloc, 0, sizeof(reloc));
> > +       memset(&execbuf, 0, sizeof(execbuf));
> > +
> > +       /* use invalid reloc to save request emission */
> > +       old_reloc = obj->relocs_ptr;
> > +       old_count = obj->relocation_count;
> > +       obj->relocs_ptr = to_user_pointer(&reloc);
> > +       obj->relocation_count = 1;
> > +       gem_write(fd, obj->handle, 0, &bbe, sizeof(bbe));
> 
> Don't use relocations for anything new. Also, don't depend on such
> quirk behavior as to kernel validating parameters in certain order.

Relocations cannot happen before reservation. Even if you were to do the
entire thing async, you still have to copy the relocation arrays from
userspace. (Note touching the relocation array would be a noticeable
performance regression.)
-Chris