[RFC PATCH i-g-t v3 2/4] lib: Add GEM minimum page size helper

[Janusz Krzysztofik]

Hi Chris,

On Wednesday, October 30, 2019 11:48:22 AM CET Chris Wilson wrote:
> Quoting Janusz Krzysztofik (2019-10-30 10:38:48)
> > Some tests assume 4kB page size 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 page size and returns
> > the size order.  Tests may use it to calculate softpin offsets suitable
> > for actually used backing store.
> > 
> > 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>
> > ---
> >  lib/ioctl_wrappers.c | 82 ++++++++++++++++++++++++++++++++++++++++++++
> >  lib/ioctl_wrappers.h |  1 +
> >  2 files changed, 83 insertions(+)
> > 
> > diff --git a/lib/ioctl_wrappers.c b/lib/ioctl_wrappers.c
> > index 280fdd62..a4313832 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
> > @@ -1157,6 +1158,87 @@ bool gem_has_softpin(int fd)
> >         return has_softpin;
> >  }
> >  
> > +static int __min_page_size_order(int fd, struct drm_i915_gem_exec_object2 
*obj,
> > +                                struct drm_i915_gem_execbuffer2 *eb,
> > +                                uint64_t offset, int min_order, int 
max_order)
> > +{
> > +       static const uint32_t bbe = MI_BATCH_BUFFER_END;
> > +       uint64_t page_size = 1ull << max_order;
> > +       int order;
> > +
> > +       if (max_order > min_order) {
> > +               /* explore upper half of the max_order at offset area */
> > +               order = __min_page_size_order(fd, obj, eb, offset, 
min_order,
> > +                                             max_order - 1);
> > +               if (order < max_order)
> > +                       return order;
> > +       }
> > +
> > +       obj->offset = gen8_canonical_addr(offset - page_size);
> > +       gem_write(fd, obj->handle, 0, &bbe, sizeof(bbe));
> > +       if (!__gem_execbuf(fd, eb)) {
> > +               /* upper half not occupied, must be the minimum */
> > +               igt_debug("found min page size=%#llx, size order=%d\n",
> > +                         (long long)page_size, max_order);
> > +               return max_order;
> > +       }
> > +
> > +       if (max_order > min_order) {
> > +               /* explore lower half of in case the upper half was 
occupied */
> > +               page_size >>= 1;
> > +               order = __min_page_size_order(fd, obj, eb, offset - 
page_size,
> > +                                             min_order, max_order - 1);
> > +               if (order < max_order)
> > +                       return order;
> > +       }
> > +
> > +       return max_order + 1;
> > +}
> > +
> > +/**
> > + * gem_min_page_size_order:
> > + * @fd: open i915 drm file descriptor
> > + *
> > + * This function detects the minimum size of a 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 size order as more handy.
> > + *
> > + * Returns:
> > + * Size order of the minimum page size
> > + */
> > +int gem_min_page_size_order(int fd)
> > +{
> > +       struct drm_i915_gem_exec_object2 obj;
> > +       struct drm_i915_gem_execbuffer2 eb;
> > +       uint64_t gtt_size = gem_aperture_size(fd);
> > +       int min_order = 12;     /* current I915_GTT_PAGE_SIZE equivalent 
*/
> > +       uint64_t page_size = 1ull << min_order;
> > +       int max_order = 21;     /* current I915_GTT_MAX_PAGE_SIZE 
equivalent */
> > +
> > +       /* no softpin => 4kB page size */
> > +       if (!gem_has_softpin(fd))
> > +               return min_order;
> > +
> > +       memset(&obj, 0, sizeof(obj));
> > +       memset(&eb, 0, sizeof(eb));
> > +
> > +       obj.handle = gem_create(fd, page_size);
> > +       obj.flags = EXEC_OBJECT_PINNED | EXEC_OBJECT_SUPPORTS_48B_ADDRESS;
> > +       eb.buffers_ptr = to_user_pointer(&obj);
> > +       eb.buffer_count = 1;
> 
> for (int order = 12; order < 64; order++) {
> 	obj.offset = 1ull << order;
> 	if (__gem_execbuf(&eb) == 0)
> 		break;
> }

Your approach seems to not take into account that addresses may be possibly 
occupied by other users.  If that happens, result will be incorrect.  Am I 
missing something?

Thanks,
Janusz


> igt_assert(obj.offset < gem_aperture_size(fd));
> gem_close(obj.handle);
> return obj.offset;
> 
> Our primary metric here is min gtt alignment => gem_gtt_min_alignment();
> -Chris
>