Re: [PATCH i-g-t v2] lib/i915: Restrict mmap types to GTT if no MMAP_OFFSET support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.winiarski@xxxxxxxxx>
> Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik@xxxxxxxxxxxxxxx>
> Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> ---
>  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@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux