Hi Chris,
On Friday, November 1, 2019 11:08:45 AM CET Chris Wilson wrote:
> Quoting Janusz Krzysztofik (2019-10-31 15:28:55)
> > 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 if incorrect detection method is used, 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 for actually used backing
> > store.
> >
> > 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, then
> > - simplify the code (Chris).
> >
> > Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik@xxxxxxxxxxxxxxx>
> > Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> > Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@xxxxxxxxx>
> > Cc: Stuart Summers <stuart.summers@xxxxxxxxx>
> > ---
> > lib/ioctl_wrappers.c | 46 ++++++++++++++++++++++++++++++++++++++++++++
> > lib/ioctl_wrappers.h | 2 ++
> > 2 files changed, 48 insertions(+)
> >
> > diff --git a/lib/ioctl_wrappers.c b/lib/ioctl_wrappers.c
> > index 628f8b83..f0ef8b2e 100644
> > --- a/lib/ioctl_wrappers.c
> > +++ b/lib/ioctl_wrappers.c
> > @@ -54,6 +54,7 @@
> > #include "intel_io.h"
> > #include "igt_debugfs.h"
> > #include "igt_sysfs.h"
> > +#include "igt_x86.h"
> > #include "config.h"
> >
> > #ifdef HAVE_VALGRIND
> > @@ -1158,6 +1159,51 @@ bool gem_has_softpin(int fd)
> > return has_softpin;
> > }
> >
> > +/**
> > + * gem_gtt_min_alignment_order:
> > + * @fd: open i915 drm file descriptor
> > + *
> > + * This function detects the minimum possible alignment of a soft-pinned
gem
> > + * object allocated from a default backing store. It is useful for
calculating
> > + * correctly aligned softpin offsets.
> > + * Since size order to size conversion (size = 1 << order) is less
trivial
> > + * than the opposite, the function returns the alignment order as more
handy.
> > + *
> > + * Returns:
> > + * Size order of the minimum GTT alignment
> > + */
> > +int gem_gtt_min_alignment_order(int fd)
>
> But not part of ioctl_wrappers.c!
>
> lib/i915/gem_gtt_topology.c? _query.c? _probe.c?
I was thinking about that but couldn't find a good name. I'll use one of your
proposed, thanks.
> > +{
> > + struct drm_i915_gem_exec_object2 obj;
> > + struct drm_i915_gem_execbuffer2 eb;
> > + const uint32_t bbe = MI_BATCH_BUFFER_END;
> > + int order;
> > +
> > + /* no softpin => 4kB page size */
> > + if (!gem_has_softpin(fd))
> > + return 12;
> > +
> > + memset(&obj, 0, sizeof(obj));
> > + memset(&eb, 0, sizeof(eb));
> > +
> > + obj.handle = gem_create(fd, 4096);
> > + obj.flags = EXEC_OBJECT_PINNED | EXEC_OBJECT_SUPPORTS_48B_ADDRESS;
> > + eb.buffers_ptr = to_user_pointer(&obj);
> > + eb.buffer_count = 1;
> > + gem_write(fd, obj.handle, 0, &bbe, sizeof(bbe));
> > +
> > + for (order = 12; order < 64; order++) {
> > + obj.offset = 1ull << order;
> > + if (__gem_execbuf(fd, &eb) != -EINVAL)
> > + break;
Should I also follow your advice of checking for -ENOSPC here rather than
!-EINVAL?
> > + }
> > + igt_assert(obj.offset < gem_aperture_size(fd));
>
> Hmm, there is one more trick we can apply here: an invalid reloc.
>
> memset(&reloc, 0, sizeof(reloc));
> obj.relocs_ptr = to_user_pointer(&reloc);
> obj.relocation_count = 1;
>
> We first process the buffers, then we process the relocations. So we
> will get the -EINVAL for illegal softpin before we get the -ENOENT for
> invalid reloc handle.
>
> That just saves actually emitting a request.
I like this idea.
Thanks,
Janusz
> -Chris
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
