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@xxxxxxxxxxxxxxx>
> Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@xxxxxxxxx>
> Cc: Stuart Summers <stuart.summers@xxxxxxxxx>
<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.
> +int gem_gtt_min_alignment_order(int fd)
> +{
> + struct drm_i915_gem_exec_object2 obj;
> + int order;
> +
> + /* no softpin => 4kB page size */
> + if (!gem_has_softpin(fd))
> + return 12;
> +
> + memset(&obj, 0, sizeof(obj));
> +
> + obj.handle = gem_create(fd, 4096);
> + obj.flags = EXEC_OBJECT_PINNED | EXEC_OBJECT_SUPPORTS_48B_ADDRESS;
Default will be system memory, so it's unclear what this would establish.
Just use PCI ID to detect this. It's not going to be dynamic within a
device. This just adds extra startup cost for detecting something that
is not dynamic.
> +/*
> + * Copyright © 2019 Intel Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
> + * IN THE SOFTWARE.
> + *
> + * Authors:
> + * Eric Anholt <eric@xxxxxxxxxx>
> + * Daniel Vetter <daniel.vetter@xxxxxxxx>
> + *
> + */
Please don't blindly copy "Authors:" around in the code.
Regards, Joonas
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
