On Monday, November 4, 2019 10:28:37 AM CET Chris Wilson wrote: > Quoting Janusz Krzysztofik (2019-11-04 09:23:12) > > 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.krzyszto...@linux.intel.com> > > > > Cc: Chris Wilson <ch...@chris-wilson.co.uk> > > > > Cc: Daniele Ceraolo Spurio <daniele.ceraolospu...@intel.com> > > > > Cc: Stuart Summers <stuart.summ...@intel.com> > > > > --- > > > > 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? > > This time we expect EINVAL for the invalid non-aligned offset provided by > the user. -ENOSPC comes later when we fail to evict -- but we will only > try that with a valid GTT node.
OK, I thought that was more about proper handling of possible error codes other than -EINVAL or -ENOSPC. Thanks, Janusz > -Chris > _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx