Please ignore this submission, the code is broken. Sorry for that.
Janusz
On Thursday, February 20, 2020 6:08:19 PM CET Janusz Krzysztofik wrote:
> Commit b0da8bb705c0 ("lib/i915: for_each_mmap_offset_type()")
> introduced a macro that makes it easy to repeat a test body within a
> loop for each mmap-offset mapping type supported by v4 of i915 MMAP_GTT
> API. However, when run on an older version of the driver, those
> subtests are believed to be still repeated for each known mmap-offset
> mapping type while effectively exercising GTT mapping type only. As
> that may be confusing, fix it.
>
> It has been assumed that the modified macro is still suitable for use
> inside gem_mmap_offset test itself. Would that not be case,
> gem_mmap_offset could redefine the macro back to its initial form for
> internal use.
>
> v2: Move extra condition to a separate function and call it via
> for_each_if(), in case we need to fix it again in future (Chris)
>
> Suggested-by: Michał Winiarski <michal.winiar...@intel.com>
> Signed-off-by: Janusz Krzysztofik <janusz.krzyszto...@linux.intel.com>
> Cc: Chris Wilson <ch...@chris-wilson.co.uk>
> ---
> lib/i915/gem_mman.c | 5 +++++
> lib/i915/gem_mman.h | 7 +++++--
> tests/i915/gem_ctx_sseu.c | 2 +-
> tests/i915/gem_exec_params.c | 2 +-
> tests/i915/gem_madvise.c | 18 ++++++++++++++----
> tests/i915/gem_mmap_offset.c | 10 +++++-----
> tests/i915/i915_pm_rpm.c | 2 +-
> 7 files changed, 32 insertions(+), 14 deletions(-)
>
> diff --git a/lib/i915/gem_mman.c b/lib/i915/gem_mman.c
> index 08ae67696..6fa8d1e8b 100644
> --- a/lib/i915/gem_mman.c
> +++ b/lib/i915/gem_mman.c
> @@ -60,6 +60,11 @@ bool gem_has_mmap_offset(int fd)
> return gtt_version >= 4;
> }
>
> +bool gem_has_mmap_offset_type(int fd, const struct mmap_offset *t);
> +{
> + return gem_has_mmap_offset(fd) || t->type == I915_MMAP_OFFSET_GTT;
> +}
> +
> /**
> * __gem_mmap__gtt:
> * @fd: open i915 drm file descriptor
> diff --git a/lib/i915/gem_mman.h b/lib/i915/gem_mman.h
> index 4fc6a0186..2c4a7a00b 100644
> --- a/lib/i915/gem_mman.h
> +++ b/lib/i915/gem_mman.h
> @@ -101,10 +101,13 @@ extern const struct mmap_offset {
> unsigned int domain;
> } mmap_offset_types[];
>
> -#define for_each_mmap_offset_type(__t) \
> +bool gem_has_mmap_offset_type(int fd, const struct mmap_offset *t);
> +
> +#define for_each_mmap_offset_type(fd, __t) \
> for (const struct mmap_offset *__t = mmap_offset_types; \
> (__t)->name; \
> - (__t)++)
> + (__t)++) \
> + for_each_if(gem_has_mmap_offset_type((fd), (__t)))
>
> #endif /* GEM_MMAN_H */
>
> diff --git a/tests/i915/gem_ctx_sseu.c b/tests/i915/gem_ctx_sseu.c
> index d558c8baa..3bef11b51 100644
> --- a/tests/i915/gem_ctx_sseu.c
> +++ b/tests/i915/gem_ctx_sseu.c
> @@ -531,7 +531,7 @@ igt_main
> test_invalid_sseu(fd);
>
> igt_subtest_with_dynamic("mmap-args") {
> - for_each_mmap_offset_type(t) {
> + for_each_mmap_offset_type(fd, t) {
> igt_dynamic_f("%s", t->name)
> test_mmapped_args(fd, t);
> }
> diff --git a/tests/i915/gem_exec_params.c b/tests/i915/gem_exec_params.c
> index e2912685b..cf7ea3065 100644
> --- a/tests/i915/gem_exec_params.c
> +++ b/tests/i915/gem_exec_params.c
> @@ -244,7 +244,7 @@ static void mmapped(int i915)
> buf = gem_create(i915, 4096);
> handle = batch_create(i915);
>
> - for_each_mmap_offset_type(t) { /* repetitive! */
> + for_each_mmap_offset_type(i915, t) { /* repetitive! */
> struct drm_i915_gem_execbuffer2 *execbuf;
> struct drm_i915_gem_exec_object2 *exec;
>
> diff --git a/tests/i915/gem_madvise.c b/tests/i915/gem_madvise.c
> index e8716a891..54c9befff 100644
> --- a/tests/i915/gem_madvise.c
> +++ b/tests/i915/gem_madvise.c
> @@ -62,12 +62,13 @@ dontneed_before_mmap(void)
> char *ptr;
> int fd;
>
> - for_each_mmap_offset_type(t) {
> + fd = drm_open_driver(DRIVER_INTEL);
> +
> + for_each_mmap_offset_type(fd, t) {
> sighandler_t old_sigsegv, old_sigbus;
>
> igt_debug("Mapping mode: %s\n", t->name);
>
> - fd = drm_open_driver(DRIVER_INTEL);
> handle = gem_create(fd, OBJECT_SIZE);
> gem_madvise(fd, handle, I915_MADV_DONTNEED);
>
> @@ -93,7 +94,11 @@ dontneed_before_mmap(void)
> munmap(ptr, OBJECT_SIZE);
> signal(SIGBUS, old_sigsegv);
> signal(SIGSEGV, old_sigbus);
> +
> + fd = drm_open_driver(DRIVER_INTEL);
> }
> +
> + close(fd);
> }
>
> static void
> @@ -103,12 +108,13 @@ dontneed_after_mmap(void)
> char *ptr;
> int fd;
>
> - for_each_mmap_offset_type(t) {
> + fd = drm_open_driver(DRIVER_INTEL);
> +
> + for_each_mmap_offset_type(fd, t) {
> sighandler_t old_sigsegv, old_sigbus;
>
> igt_debug("Mapping mode: %s\n", t->name);
>
> - fd = drm_open_driver(DRIVER_INTEL);
> handle = gem_create(fd, OBJECT_SIZE);
>
> ptr = __gem_mmap_offset(fd, handle, 0, OBJECT_SIZE,
> @@ -134,7 +140,11 @@ dontneed_after_mmap(void)
> munmap(ptr, OBJECT_SIZE);
> signal(SIGBUS, old_sigbus);
> signal(SIGSEGV, old_sigsegv);
> +
> + fd = drm_open_driver(DRIVER_INTEL);
> }
> +
> + close(fd);
> }
>
> static void
> diff --git a/tests/i915/gem_mmap_offset.c b/tests/i915/gem_mmap_offset.c
> index f49d18e63..1ec963b25 100644
> --- a/tests/i915/gem_mmap_offset.c
> +++ b/tests/i915/gem_mmap_offset.c
> @@ -128,7 +128,7 @@ static void basic_uaf(int i915)
> {
> const uint32_t obj_size = 4096;
>
> - for_each_mmap_offset_type(t) {
> + for_each_mmap_offset_type(i915, t) {
> uint32_t handle = gem_create(i915, obj_size);
> uint8_t *expected, *buf, *addr;
>
> @@ -176,7 +176,7 @@ static void basic_uaf(int i915)
>
> static void isolation(int i915)
> {
> - for_each_mmap_offset_type(t) {
> + for_each_mmap_offset_type(i915, t) {
> struct drm_i915_gem_mmap_offset mmap_arg = {
> .flags = t->type
> };
> @@ -245,7 +245,7 @@ static void pf_nonblock(int i915)
> {
> igt_spin_t *spin = igt_spin_new(i915);
>
> - for_each_mmap_offset_type(t) {
> + for_each_mmap_offset_type(i915, t) {
> uint32_t *ptr;
>
> ptr = __mmap_offset(i915, spin->handle, 0, 4096,
> @@ -324,7 +324,7 @@ static void open_flood(int i915, int timeout)
> handle = gem_create(i915, 4096);
> dmabuf = prime_handle_to_fd(i915, handle);
>
> - for_each_mmap_offset_type(t) {
> + for_each_mmap_offset_type(i915, t) {
> struct drm_i915_gem_mmap_offset arg = {
> .handle = handle,
> .flags = t->type,
> @@ -351,7 +351,7 @@ static void open_flood(int i915, int timeout)
> tmp = gem_reopen_driver(i915);
> handle = prime_fd_to_handle(i915, dmabuf);
>
> - for_each_mmap_offset_type(t) {
> + for_each_mmap_offset_type(i915, t) {
> struct drm_i915_gem_mmap_offset arg = {
> .handle = handle,
> .flags = t->type,
> diff --git a/tests/i915/i915_pm_rpm.c b/tests/i915/i915_pm_rpm.c
> index 0c2821122..1bec80db7 100644
> --- a/tests/i915/i915_pm_rpm.c
> +++ b/tests/i915/i915_pm_rpm.c
> @@ -2006,7 +2006,7 @@ igt_main_args("", long_options, help_str, opt_handler,
> NULL)
>
> /* GEM */
> igt_subtest_with_dynamic("gem-mmap-type") {
> - for_each_mmap_offset_type(t) {
> + for_each_mmap_offset_type(drm_fd, t) {
> igt_dynamic_f("%s", t->name)
> gem_mmap_args(t);
> }
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
