All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFT i-g-t] lib/i915: Assert mmap size alignment
@ 2019-03-04 14:11 ` Tvrtko Ursulin
  0 siblings, 0 replies; 14+ messages in thread
From: Tvrtko Ursulin @ 2019-03-04 14:11 UTC (permalink / raw)
  To: igt-dev; +Cc: Intel-gfx

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Fishing for fails...

/*
mmap(2) mandates size is page aligned so check this in our wrappers.
*/

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 lib/i915/gem_mman.c |  4 ++++
 lib/igt_fb.c        | 14 ++++++++------
 tests/kms_ccs.c     | 14 ++++++++------
 tests/kms_psr.c     |  8 ++++----
 4 files changed, 24 insertions(+), 16 deletions(-)

diff --git a/lib/i915/gem_mman.c b/lib/i915/gem_mman.c
index 3cf9a6bbdb31..084dbb3b3678 100644
--- a/lib/i915/gem_mman.c
+++ b/lib/i915/gem_mman.c
@@ -57,6 +57,8 @@ void *__gem_mmap__gtt(int fd, uint32_t handle, uint64_t size, unsigned prot)
 	struct drm_i915_gem_mmap_gtt mmap_arg;
 	void *ptr;
 
+	igt_assert(!(size & 4095));
+
 	memset(&mmap_arg, 0, sizeof(mmap_arg));
 	mmap_arg.handle = handle;
 	if (igt_ioctl(fd, DRM_IOCTL_I915_GEM_MMAP_GTT, &mmap_arg))
@@ -162,6 +164,8 @@ static void
 {
 	struct drm_i915_gem_mmap arg;
 
+	igt_assert(!(size & 4095));
+
 	memset(&arg, 0, sizeof(arg));
 	arg.handle = handle;
 	arg.offset = offset;
diff --git a/lib/igt_fb.c b/lib/igt_fb.c
index 9dca2a4603ce..82f0a41631c1 100644
--- a/lib/igt_fb.c
+++ b/lib/igt_fb.c
@@ -1494,7 +1494,7 @@ static void free_linear_mapping(struct fb_blit_upload *blit)
 	struct igt_fb *fb = blit->fb;
 	struct fb_blit_linear *linear = &blit->linear;
 
-	gem_munmap(linear->map, linear->fb.size);
+	gem_munmap(linear->map, ALIGN(linear->fb.size, 4096));
 	gem_set_domain(fd, linear->fb.gem_handle,
 		       I915_GEM_DOMAIN_GTT, 0);
 
@@ -1544,7 +1544,8 @@ static void setup_linear_mapping(int fd, struct igt_fb *fb, struct fb_blit_linea
 
 	/* Setup cairo context */
 	linear->map = gem_mmap__cpu(fd, linear->fb.gem_handle,
-				    0, linear->fb.size, PROT_READ | PROT_WRITE);
+				    0, ALIGN(linear->fb.size, 4096),
+				    PROT_READ | PROT_WRITE);
 }
 
 static void create_cairo_surface__blit(int fd, struct igt_fb *fb)
@@ -1588,7 +1589,7 @@ int igt_dirty_fb(int fd, struct igt_fb *fb)
 
 static void unmap_bo(struct igt_fb *fb, void *ptr)
 {
-	gem_munmap(ptr, fb->size);
+	gem_munmap(ptr, ALIGN(fb->size, 4096));
 
 	if (fb->is_dumb)
 		igt_dirty_fb(fb->fd, fb);
@@ -1604,6 +1605,7 @@ static void destroy_cairo_surface__gtt(void *arg)
 
 static void *map_bo(int fd, struct igt_fb *fb)
 {
+	size_t size = ALIGN(fb->size, 4096);
 	void *ptr;
 
 	if (is_i915_device(fd))
@@ -1611,13 +1613,13 @@ static void *map_bo(int fd, struct igt_fb *fb)
 			       I915_GEM_DOMAIN_GTT, I915_GEM_DOMAIN_GTT);
 
 	if (fb->is_dumb)
-		ptr = kmstest_dumb_map_buffer(fd, fb->gem_handle, fb->size,
+		ptr = kmstest_dumb_map_buffer(fd, fb->gem_handle, size,
 					      PROT_READ | PROT_WRITE);
 	else if (is_i915_device(fd))
-		ptr = gem_mmap__gtt(fd, fb->gem_handle, fb->size,
+		ptr = gem_mmap__gtt(fd, fb->gem_handle, size,
 				    PROT_READ | PROT_WRITE);
 	else if (is_vc4_device(fd))
-		ptr = igt_vc4_mmap_bo(fd, fb->gem_handle, fb->size,
+		ptr = igt_vc4_mmap_bo(fd, fb->gem_handle, size,
 				      PROT_READ | PROT_WRITE);
 	else
 		igt_assert(false);
diff --git a/tests/kms_ccs.c b/tests/kms_ccs.c
index 1ed2b4a08c5d..1b92527ee50d 100644
--- a/tests/kms_ccs.c
+++ b/tests/kms_ccs.c
@@ -196,15 +196,16 @@ static void render_fb(data_t *data, uint32_t gem_handle, unsigned int size,
 		      enum test_fb_flags fb_flags,
 		      int height, unsigned int stride)
 {
-	uint32_t *ptr;
-	unsigned int half_height, half_size;
 	uint32_t uncompressed_color = data->plane ? GREEN : RED;
 	uint32_t compressed_color =
 		data->plane ? COMPRESSED_GREEN : COMPRESSED_RED;
+	size_t mmap_size = ALIGN(size, 4096);
 	uint32_t bad_color = RED;
+	unsigned int half_height, half_size;
+	uint32_t *ptr;
 	int i;
 
-	ptr = gem_mmap__cpu(data->drm_fd, gem_handle, 0, size,
+	ptr = gem_mmap__cpu(data->drm_fd, gem_handle, 0, mmap_size,
 			    PROT_READ | PROT_WRITE);
 
 	if (fb_flags & FB_COMPRESSED) {
@@ -243,7 +244,7 @@ static void render_fb(data_t *data, uint32_t gem_handle, unsigned int size,
 		}
 	}
 
-	munmap(ptr, size);
+	munmap(ptr, mmap_size);
 }
 
 static unsigned int
@@ -259,6 +260,7 @@ static void render_ccs(data_t *data, uint32_t gem_handle,
 		       uint32_t offset, uint32_t size,
 		       int height, unsigned int ccs_stride)
 {
+	size_t mmap_size = ALIGN(size, 4096);
 	unsigned int half_height, ccs_half_height;
 	uint8_t *ptr;
 	int i;
@@ -266,7 +268,7 @@ static void render_ccs(data_t *data, uint32_t gem_handle,
 	half_height = ALIGN(height, 128) / 2;
 	ccs_half_height = half_height / 16;
 
-	ptr = gem_mmap__cpu(data->drm_fd, gem_handle, offset, size,
+	ptr = gem_mmap__cpu(data->drm_fd, gem_handle, offset, mmap_size,
 			    PROT_READ | PROT_WRITE);
 
 	for (i = 0; i < size; i++) {
@@ -276,7 +278,7 @@ static void render_ccs(data_t *data, uint32_t gem_handle,
 			ptr[i] = CCS_COMPRESSED;
 	}
 
-	munmap(ptr, size);
+	munmap(ptr, mmap_size);
 }
 
 static void generate_fb(data_t *data, struct igt_fb *fb,
diff --git a/tests/kms_psr.c b/tests/kms_psr.c
index 3e16a6bf4f37..5d3f0ed87eec 100644
--- a/tests/kms_psr.c
+++ b/tests/kms_psr.c
@@ -270,8 +270,8 @@ static void run_test(data_t *data)
 		expected = "GREEN";
 		break;
 	case MMAP_GTT:
-		ptr = gem_mmap__gtt(data->drm_fd, handle, data->mod_size,
-				    PROT_WRITE);
+		ptr = gem_mmap__gtt(data->drm_fd, handle,
+				    ALIGN(data->mod_size, 4096), PROT_WRITE);
 		gem_set_domain(data->drm_fd, handle,
 			       I915_GEM_DOMAIN_GTT, I915_GEM_DOMAIN_GTT);
 		memset(ptr, 0xcc, data->mod_size);
@@ -279,8 +279,8 @@ static void run_test(data_t *data)
 		expected = "BLACK or TRANSPARENT mark on top of plane in test";
 		break;
 	case MMAP_CPU:
-		ptr = gem_mmap__cpu(data->drm_fd, handle, 0, data->mod_size,
-				    PROT_WRITE);
+		ptr = gem_mmap__cpu(data->drm_fd, handle, 0,
+				    ALIGN(data->mod_size, 4096), PROT_WRITE);
 		gem_set_domain(data->drm_fd, handle,
 			       I915_GEM_DOMAIN_CPU, I915_GEM_DOMAIN_CPU);
 		memset(ptr, 0, data->mod_size);
-- 
2.19.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [igt-dev] [RFT i-g-t] lib/i915: Assert mmap size alignment
@ 2019-03-04 14:11 ` Tvrtko Ursulin
  0 siblings, 0 replies; 14+ messages in thread
From: Tvrtko Ursulin @ 2019-03-04 14:11 UTC (permalink / raw)
  To: igt-dev; +Cc: Intel-gfx, Tvrtko Ursulin

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Fishing for fails...

/*
mmap(2) mandates size is page aligned so check this in our wrappers.
*/

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 lib/i915/gem_mman.c |  4 ++++
 lib/igt_fb.c        | 14 ++++++++------
 tests/kms_ccs.c     | 14 ++++++++------
 tests/kms_psr.c     |  8 ++++----
 4 files changed, 24 insertions(+), 16 deletions(-)

diff --git a/lib/i915/gem_mman.c b/lib/i915/gem_mman.c
index 3cf9a6bbdb31..084dbb3b3678 100644
--- a/lib/i915/gem_mman.c
+++ b/lib/i915/gem_mman.c
@@ -57,6 +57,8 @@ void *__gem_mmap__gtt(int fd, uint32_t handle, uint64_t size, unsigned prot)
 	struct drm_i915_gem_mmap_gtt mmap_arg;
 	void *ptr;
 
+	igt_assert(!(size & 4095));
+
 	memset(&mmap_arg, 0, sizeof(mmap_arg));
 	mmap_arg.handle = handle;
 	if (igt_ioctl(fd, DRM_IOCTL_I915_GEM_MMAP_GTT, &mmap_arg))
@@ -162,6 +164,8 @@ static void
 {
 	struct drm_i915_gem_mmap arg;
 
+	igt_assert(!(size & 4095));
+
 	memset(&arg, 0, sizeof(arg));
 	arg.handle = handle;
 	arg.offset = offset;
diff --git a/lib/igt_fb.c b/lib/igt_fb.c
index 9dca2a4603ce..82f0a41631c1 100644
--- a/lib/igt_fb.c
+++ b/lib/igt_fb.c
@@ -1494,7 +1494,7 @@ static void free_linear_mapping(struct fb_blit_upload *blit)
 	struct igt_fb *fb = blit->fb;
 	struct fb_blit_linear *linear = &blit->linear;
 
-	gem_munmap(linear->map, linear->fb.size);
+	gem_munmap(linear->map, ALIGN(linear->fb.size, 4096));
 	gem_set_domain(fd, linear->fb.gem_handle,
 		       I915_GEM_DOMAIN_GTT, 0);
 
@@ -1544,7 +1544,8 @@ static void setup_linear_mapping(int fd, struct igt_fb *fb, struct fb_blit_linea
 
 	/* Setup cairo context */
 	linear->map = gem_mmap__cpu(fd, linear->fb.gem_handle,
-				    0, linear->fb.size, PROT_READ | PROT_WRITE);
+				    0, ALIGN(linear->fb.size, 4096),
+				    PROT_READ | PROT_WRITE);
 }
 
 static void create_cairo_surface__blit(int fd, struct igt_fb *fb)
@@ -1588,7 +1589,7 @@ int igt_dirty_fb(int fd, struct igt_fb *fb)
 
 static void unmap_bo(struct igt_fb *fb, void *ptr)
 {
-	gem_munmap(ptr, fb->size);
+	gem_munmap(ptr, ALIGN(fb->size, 4096));
 
 	if (fb->is_dumb)
 		igt_dirty_fb(fb->fd, fb);
@@ -1604,6 +1605,7 @@ static void destroy_cairo_surface__gtt(void *arg)
 
 static void *map_bo(int fd, struct igt_fb *fb)
 {
+	size_t size = ALIGN(fb->size, 4096);
 	void *ptr;
 
 	if (is_i915_device(fd))
@@ -1611,13 +1613,13 @@ static void *map_bo(int fd, struct igt_fb *fb)
 			       I915_GEM_DOMAIN_GTT, I915_GEM_DOMAIN_GTT);
 
 	if (fb->is_dumb)
-		ptr = kmstest_dumb_map_buffer(fd, fb->gem_handle, fb->size,
+		ptr = kmstest_dumb_map_buffer(fd, fb->gem_handle, size,
 					      PROT_READ | PROT_WRITE);
 	else if (is_i915_device(fd))
-		ptr = gem_mmap__gtt(fd, fb->gem_handle, fb->size,
+		ptr = gem_mmap__gtt(fd, fb->gem_handle, size,
 				    PROT_READ | PROT_WRITE);
 	else if (is_vc4_device(fd))
-		ptr = igt_vc4_mmap_bo(fd, fb->gem_handle, fb->size,
+		ptr = igt_vc4_mmap_bo(fd, fb->gem_handle, size,
 				      PROT_READ | PROT_WRITE);
 	else
 		igt_assert(false);
diff --git a/tests/kms_ccs.c b/tests/kms_ccs.c
index 1ed2b4a08c5d..1b92527ee50d 100644
--- a/tests/kms_ccs.c
+++ b/tests/kms_ccs.c
@@ -196,15 +196,16 @@ static void render_fb(data_t *data, uint32_t gem_handle, unsigned int size,
 		      enum test_fb_flags fb_flags,
 		      int height, unsigned int stride)
 {
-	uint32_t *ptr;
-	unsigned int half_height, half_size;
 	uint32_t uncompressed_color = data->plane ? GREEN : RED;
 	uint32_t compressed_color =
 		data->plane ? COMPRESSED_GREEN : COMPRESSED_RED;
+	size_t mmap_size = ALIGN(size, 4096);
 	uint32_t bad_color = RED;
+	unsigned int half_height, half_size;
+	uint32_t *ptr;
 	int i;
 
-	ptr = gem_mmap__cpu(data->drm_fd, gem_handle, 0, size,
+	ptr = gem_mmap__cpu(data->drm_fd, gem_handle, 0, mmap_size,
 			    PROT_READ | PROT_WRITE);
 
 	if (fb_flags & FB_COMPRESSED) {
@@ -243,7 +244,7 @@ static void render_fb(data_t *data, uint32_t gem_handle, unsigned int size,
 		}
 	}
 
-	munmap(ptr, size);
+	munmap(ptr, mmap_size);
 }
 
 static unsigned int
@@ -259,6 +260,7 @@ static void render_ccs(data_t *data, uint32_t gem_handle,
 		       uint32_t offset, uint32_t size,
 		       int height, unsigned int ccs_stride)
 {
+	size_t mmap_size = ALIGN(size, 4096);
 	unsigned int half_height, ccs_half_height;
 	uint8_t *ptr;
 	int i;
@@ -266,7 +268,7 @@ static void render_ccs(data_t *data, uint32_t gem_handle,
 	half_height = ALIGN(height, 128) / 2;
 	ccs_half_height = half_height / 16;
 
-	ptr = gem_mmap__cpu(data->drm_fd, gem_handle, offset, size,
+	ptr = gem_mmap__cpu(data->drm_fd, gem_handle, offset, mmap_size,
 			    PROT_READ | PROT_WRITE);
 
 	for (i = 0; i < size; i++) {
@@ -276,7 +278,7 @@ static void render_ccs(data_t *data, uint32_t gem_handle,
 			ptr[i] = CCS_COMPRESSED;
 	}
 
-	munmap(ptr, size);
+	munmap(ptr, mmap_size);
 }
 
 static void generate_fb(data_t *data, struct igt_fb *fb,
diff --git a/tests/kms_psr.c b/tests/kms_psr.c
index 3e16a6bf4f37..5d3f0ed87eec 100644
--- a/tests/kms_psr.c
+++ b/tests/kms_psr.c
@@ -270,8 +270,8 @@ static void run_test(data_t *data)
 		expected = "GREEN";
 		break;
 	case MMAP_GTT:
-		ptr = gem_mmap__gtt(data->drm_fd, handle, data->mod_size,
-				    PROT_WRITE);
+		ptr = gem_mmap__gtt(data->drm_fd, handle,
+				    ALIGN(data->mod_size, 4096), PROT_WRITE);
 		gem_set_domain(data->drm_fd, handle,
 			       I915_GEM_DOMAIN_GTT, I915_GEM_DOMAIN_GTT);
 		memset(ptr, 0xcc, data->mod_size);
@@ -279,8 +279,8 @@ static void run_test(data_t *data)
 		expected = "BLACK or TRANSPARENT mark on top of plane in test";
 		break;
 	case MMAP_CPU:
-		ptr = gem_mmap__cpu(data->drm_fd, handle, 0, data->mod_size,
-				    PROT_WRITE);
+		ptr = gem_mmap__cpu(data->drm_fd, handle, 0,
+				    ALIGN(data->mod_size, 4096), PROT_WRITE);
 		gem_set_domain(data->drm_fd, handle,
 			       I915_GEM_DOMAIN_CPU, I915_GEM_DOMAIN_CPU);
 		memset(ptr, 0, data->mod_size);
-- 
2.19.1

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [igt-dev] ✓ Fi.CI.BAT: success for lib/i915: Assert mmap size alignment (rev4)
  2019-03-04 14:11 ` [igt-dev] " Tvrtko Ursulin
  (?)
@ 2019-03-04 14:41 ` Patchwork
  -1 siblings, 0 replies; 14+ messages in thread
From: Patchwork @ 2019-03-04 14:41 UTC (permalink / raw)
  To: Tvrtko Ursulin; +Cc: igt-dev

== Series Details ==

Series: lib/i915: Assert mmap size alignment (rev4)
URL   : https://patchwork.freedesktop.org/series/57396/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_5694 -> IGTPW_2548
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/57396/revisions/4/mbox/

Known issues
------------

  Here are the changes found in IGTPW_2548 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_exec_suspend@basic-s4-devices:
    - fi-blb-e6850:       NOTRUN -> INCOMPLETE [fdo#107718]

  
#### Possible fixes ####

  * igt@gem_exec_suspend@basic-s3:
    - fi-blb-e6850:       INCOMPLETE [fdo#107718] -> PASS

  * igt@i915_pm_rpm@basic-pci-d3-state:
    - fi-byt-j1900:       SKIP [fdo#109271] -> PASS

  * igt@i915_pm_rpm@basic-rte:
    - fi-byt-j1900:       FAIL [fdo#108800] -> PASS

  * igt@kms_busy@basic-flip-b:
    - fi-gdg-551:         FAIL [fdo#103182] -> PASS

  * igt@kms_chamelium@common-hpd-after-suspend:
    - fi-kbl-7567u:       WARN [fdo#109380] -> PASS

  * igt@kms_pipe_crc_basic@nonblocking-crc-pipe-a-frame-sequence:
    - fi-byt-clapper:     FAIL [fdo#103191] / [fdo#107362] -> PASS

  * igt@kms_pipe_crc_basic@nonblocking-crc-pipe-c:
    - fi-kbl-7567u:       SKIP [fdo#109271] -> PASS +33

  
  [fdo#103182]: https://bugs.freedesktop.org/show_bug.cgi?id=103182
  [fdo#103191]: https://bugs.freedesktop.org/show_bug.cgi?id=103191
  [fdo#107362]: https://bugs.freedesktop.org/show_bug.cgi?id=107362
  [fdo#107718]: https://bugs.freedesktop.org/show_bug.cgi?id=107718
  [fdo#108800]: https://bugs.freedesktop.org/show_bug.cgi?id=108800
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109380]: https://bugs.freedesktop.org/show_bug.cgi?id=109380


Participating hosts (46 -> 40)
------------------------------

  Missing    (6): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 fi-bdw-samus 


Build changes
-------------

    * IGT: IGT_4869 -> IGTPW_2548

  CI_DRM_5694: daab510ef99070d4974e8408f34b465be5c3c0b7 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_2548: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2548/
  IGT_4869: a958d3f60b7718151fd0bafcdd1e4874262f51b8 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2548/
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [igt-dev] [RFT i-g-t] lib/i915: Assert mmap size alignment
  2019-03-04 14:11 ` [igt-dev] " Tvrtko Ursulin
@ 2019-03-04 14:45   ` Chris Wilson
  -1 siblings, 0 replies; 14+ messages in thread
From: Chris Wilson @ 2019-03-04 14:45 UTC (permalink / raw)
  To: Tvrtko Ursulin, igt-dev; +Cc: Intel-gfx

Quoting Tvrtko Ursulin (2019-03-04 14:11:31)
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> Fishing for fails...
> 
> /*
> mmap(2) mandates size is page aligned so check this in our wrappers.
> */
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
>  lib/i915/gem_mman.c |  4 ++++
>  lib/igt_fb.c        | 14 ++++++++------
>  tests/kms_ccs.c     | 14 ++++++++------
>  tests/kms_psr.c     |  8 ++++----
>  4 files changed, 24 insertions(+), 16 deletions(-)
> 
> diff --git a/lib/i915/gem_mman.c b/lib/i915/gem_mman.c
> index 3cf9a6bbdb31..084dbb3b3678 100644
> --- a/lib/i915/gem_mman.c
> +++ b/lib/i915/gem_mman.c
> @@ -57,6 +57,8 @@ void *__gem_mmap__gtt(int fd, uint32_t handle, uint64_t size, unsigned prot)
>         struct drm_i915_gem_mmap_gtt mmap_arg;
>         void *ptr;
>  
> +       igt_assert(!(size & 4095));

Fwiw, it's probably best if we had some kind of IGT_BUG_ON() that caused
a much more visible explosion than a mere test failure.

#define IGT_BUG_ON(cond) if (!(cond)) raise(SIGILL)
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [igt-dev] [RFT i-g-t] lib/i915: Assert mmap size alignment
@ 2019-03-04 14:45   ` Chris Wilson
  0 siblings, 0 replies; 14+ messages in thread
From: Chris Wilson @ 2019-03-04 14:45 UTC (permalink / raw)
  To: Tvrtko Ursulin, igt-dev; +Cc: Intel-gfx, Tvrtko Ursulin

Quoting Tvrtko Ursulin (2019-03-04 14:11:31)
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> Fishing for fails...
> 
> /*
> mmap(2) mandates size is page aligned so check this in our wrappers.
> */
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
>  lib/i915/gem_mman.c |  4 ++++
>  lib/igt_fb.c        | 14 ++++++++------
>  tests/kms_ccs.c     | 14 ++++++++------
>  tests/kms_psr.c     |  8 ++++----
>  4 files changed, 24 insertions(+), 16 deletions(-)
> 
> diff --git a/lib/i915/gem_mman.c b/lib/i915/gem_mman.c
> index 3cf9a6bbdb31..084dbb3b3678 100644
> --- a/lib/i915/gem_mman.c
> +++ b/lib/i915/gem_mman.c
> @@ -57,6 +57,8 @@ void *__gem_mmap__gtt(int fd, uint32_t handle, uint64_t size, unsigned prot)
>         struct drm_i915_gem_mmap_gtt mmap_arg;
>         void *ptr;
>  
> +       igt_assert(!(size & 4095));

Fwiw, it's probably best if we had some kind of IGT_BUG_ON() that caused
a much more visible explosion than a mere test failure.

#define IGT_BUG_ON(cond) if (!(cond)) raise(SIGILL)
-Chris
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [igt-dev] ✓ Fi.CI.IGT: success for lib/i915: Assert mmap size alignment (rev4)
  2019-03-04 14:11 ` [igt-dev] " Tvrtko Ursulin
                   ` (2 preceding siblings ...)
  (?)
@ 2019-03-04 18:33 ` Patchwork
  -1 siblings, 0 replies; 14+ messages in thread
From: Patchwork @ 2019-03-04 18:33 UTC (permalink / raw)
  To: Tvrtko Ursulin; +Cc: igt-dev

== Series Details ==

Series: lib/i915: Assert mmap size alignment (rev4)
URL   : https://patchwork.freedesktop.org/series/57396/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_5694_full -> IGTPW_2548_full
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/57396/revisions/4/mbox/

Known issues
------------

  Here are the changes found in IGTPW_2548_full that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_exec_parse@basic-allocation:
    - shard-glk:          NOTRUN -> SKIP [fdo#109271] +20

  * igt@gem_mocs_settings@mocs-reset-bsd2:
    - shard-snb:          NOTRUN -> SKIP [fdo#109271] +183

  * igt@gen3_render_tiledy_blits:
    - shard-kbl:          NOTRUN -> SKIP [fdo#109271] +35

  * igt@kms_busy@extended-modeset-hang-newfb-with-reset-render-a:
    - shard-hsw:          PASS -> DMESG-WARN [fdo#107956]
    - shard-kbl:          PASS -> DMESG-WARN [fdo#107956]

  * igt@kms_busy@extended-modeset-hang-newfb-with-reset-render-b:
    - shard-glk:          NOTRUN -> DMESG-WARN [fdo#107956]

  * igt@kms_busy@extended-modeset-hang-oldfb-render-e:
    - shard-glk:          NOTRUN -> SKIP [fdo#109271] / [fdo#109278]

  * igt@kms_color@pipe-c-ctm-max:
    - shard-apl:          PASS -> FAIL [fdo#108147]

  * igt@kms_content_protection@atomic-dpms:
    - shard-kbl:          NOTRUN -> FAIL [fdo#108739]

  * igt@kms_cursor_crc@cursor-256x256-suspend:
    - shard-apl:          PASS -> FAIL [fdo#103191] / [fdo#103232]

  * igt@kms_cursor_crc@cursor-64x64-dpms:
    - shard-apl:          PASS -> FAIL [fdo#103232]

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-onoff:
    - shard-kbl:          NOTRUN -> FAIL [fdo#103167]

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-pwrite:
    - shard-apl:          PASS -> FAIL [fdo#103167] +3

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-move:
    - shard-glk:          PASS -> FAIL [fdo#103167] +3

  * igt@kms_plane@pixel-format-pipe-a-planes-source-clamping:
    - shard-glk:          PASS -> FAIL [fdo#108948]
    - shard-apl:          PASS -> FAIL [fdo#108948]

  * igt@kms_plane@plane-position-covered-pipe-c-planes:
    - shard-glk:          PASS -> FAIL [fdo#103166] +3

  * igt@kms_plane_multiple@atomic-pipe-b-tiling-yf:
    - shard-kbl:          PASS -> FAIL [fdo#103166] +1

  * igt@kms_plane_multiple@atomic-pipe-c-tiling-yf:
    - shard-apl:          PASS -> FAIL [fdo#103166] +3

  * igt@kms_universal_plane@disable-primary-vs-flip-pipe-d:
    - shard-kbl:          NOTRUN -> SKIP [fdo#109271] / [fdo#109278] +3
    - shard-snb:          NOTRUN -> SKIP [fdo#109271] / [fdo#109278] +20
    - shard-apl:          NOTRUN -> SKIP [fdo#109271] / [fdo#109278]

  * igt@perf_pmu@rc6-runtime-pm-long:
    - shard-apl:          PASS -> FAIL [fdo#105010]
    - shard-kbl:          PASS -> FAIL [fdo#105010]

  * igt@prime_nv_test@i915_import_gtt_mmap:
    - shard-apl:          NOTRUN -> SKIP [fdo#109271] +4

  * igt@tools_test@sysfs_l3_parity:
    - shard-hsw:          PASS -> SKIP [fdo#109271]

  
#### Possible fixes ####

  * igt@kms_busy@extended-modeset-hang-newfb-render-b:
    - shard-hsw:          DMESG-WARN [fdo#107956] -> PASS

  * igt@kms_busy@extended-modeset-hang-newfb-render-c:
    - shard-kbl:          DMESG-WARN [fdo#107956] -> PASS

  * igt@kms_cursor_crc@cursor-128x128-sliding:
    - shard-kbl:          FAIL [fdo#103232] -> PASS +3

  * igt@kms_cursor_crc@cursor-256x85-random:
    - shard-apl:          FAIL [fdo#103232] -> PASS +5

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-mmap-cpu:
    - shard-apl:          FAIL [fdo#103167] -> PASS
    - shard-kbl:          FAIL [fdo#103167] -> PASS

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-fullscreen:
    - shard-glk:          FAIL [fdo#103167] -> PASS +3

  * igt@kms_frontbuffer_tracking@fbc-1p-rte:
    - shard-glk:          FAIL [fdo#103167] / [fdo#105682] -> PASS
    - shard-kbl:          FAIL [fdo#103167] / [fdo#105682] -> PASS
    - shard-apl:          FAIL [fdo#103167] / [fdo#105682] -> PASS

  * igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a:
    - shard-kbl:          INCOMPLETE [fdo#103665] -> PASS +1

  * igt@kms_plane_multiple@atomic-pipe-a-tiling-y:
    - shard-apl:          FAIL [fdo#103166] -> PASS +3
    - shard-kbl:          FAIL [fdo#103166] -> PASS +1

  * igt@kms_plane_multiple@atomic-pipe-b-tiling-none:
    - shard-glk:          FAIL [fdo#103166] -> PASS +3

  * igt@kms_rotation_crc@multiplane-rotation-cropping-top:
    - shard-kbl:          FAIL [fdo#109016] -> PASS

  * igt@kms_vblank@pipe-c-ts-continuation-dpms-suspend:
    - shard-apl:          FAIL [fdo#104894] -> PASS +1
    - shard-kbl:          FAIL [fdo#104894] -> PASS

  
#### Warnings ####

  * igt@kms_rotation_crc@multiplane-rotation:
    - shard-kbl:          INCOMPLETE [fdo#103665] -> FAIL [fdo#109016]

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#103166]: https://bugs.freedesktop.org/show_bug.cgi?id=103166
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103191]: https://bugs.freedesktop.org/show_bug.cgi?id=103191
  [fdo#103232]: https://bugs.freedesktop.org/show_bug.cgi?id=103232
  [fdo#103665]: https://bugs.freedesktop.org/show_bug.cgi?id=103665
  [fdo#104894]: https://bugs.freedesktop.org/show_bug.cgi?id=104894
  [fdo#105010]: https://bugs.freedesktop.org/show_bug.cgi?id=105010
  [fdo#105682]: https://bugs.freedesktop.org/show_bug.cgi?id=105682
  [fdo#107956]: https://bugs.freedesktop.org/show_bug.cgi?id=107956
  [fdo#108147]: https://bugs.freedesktop.org/show_bug.cgi?id=108147
  [fdo#108739]: https://bugs.freedesktop.org/show_bug.cgi?id=108739
  [fdo#108948]: https://bugs.freedesktop.org/show_bug.cgi?id=108948
  [fdo#109016]: https://bugs.freedesktop.org/show_bug.cgi?id=109016
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278


Participating hosts (7 -> 5)
------------------------------

  Missing    (2): shard-skl shard-iclb 


Build changes
-------------

    * IGT: IGT_4869 -> IGTPW_2548
    * Piglit: piglit_4509 -> None

  CI_DRM_5694: daab510ef99070d4974e8408f34b465be5c3c0b7 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_2548: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2548/
  IGT_4869: a958d3f60b7718151fd0bafcdd1e4874262f51b8 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2548/
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [igt-dev] [RFT i-g-t] lib/i915: Assert mmap size alignment
  2019-03-04 14:45   ` Chris Wilson
@ 2019-03-05  7:55     ` Tvrtko Ursulin
  -1 siblings, 0 replies; 14+ messages in thread
From: Tvrtko Ursulin @ 2019-03-05  7:55 UTC (permalink / raw)
  To: Chris Wilson, igt-dev; +Cc: Intel-gfx


On 04/03/2019 14:45, Chris Wilson wrote:
> Quoting Tvrtko Ursulin (2019-03-04 14:11:31)
>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>
>> Fishing for fails...
>>
>> /*
>> mmap(2) mandates size is page aligned so check this in our wrappers.
>> */
>>
>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>> ---
>>   lib/i915/gem_mman.c |  4 ++++
>>   lib/igt_fb.c        | 14 ++++++++------
>>   tests/kms_ccs.c     | 14 ++++++++------
>>   tests/kms_psr.c     |  8 ++++----
>>   4 files changed, 24 insertions(+), 16 deletions(-)
>>
>> diff --git a/lib/i915/gem_mman.c b/lib/i915/gem_mman.c
>> index 3cf9a6bbdb31..084dbb3b3678 100644
>> --- a/lib/i915/gem_mman.c
>> +++ b/lib/i915/gem_mman.c
>> @@ -57,6 +57,8 @@ void *__gem_mmap__gtt(int fd, uint32_t handle, uint64_t size, unsigned prot)
>>          struct drm_i915_gem_mmap_gtt mmap_arg;
>>          void *ptr;
>>   
>> +       igt_assert(!(size & 4095));
> 
> Fwiw, it's probably best if we had some kind of IGT_BUG_ON() that caused
> a much more visible explosion than a mere test failure.
> 
> #define IGT_BUG_ON(cond) if (!(cond)) raise(SIGILL)

No idea if that would be worthwhile.

But anyway, it seems I have identified all the tests which do 
non-page-aligned mmap. So..

a) leave the assert in low level mmap helpers
b) move the assert to higher level wrappers
c) round up in wrappers is not an option due unmap
d) do nothing in igt

?

Regards,

Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [igt-dev] [RFT i-g-t] lib/i915: Assert mmap size alignment
@ 2019-03-05  7:55     ` Tvrtko Ursulin
  0 siblings, 0 replies; 14+ messages in thread
From: Tvrtko Ursulin @ 2019-03-05  7:55 UTC (permalink / raw)
  To: Chris Wilson, igt-dev; +Cc: Intel-gfx, Tvrtko Ursulin


On 04/03/2019 14:45, Chris Wilson wrote:
> Quoting Tvrtko Ursulin (2019-03-04 14:11:31)
>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>
>> Fishing for fails...
>>
>> /*
>> mmap(2) mandates size is page aligned so check this in our wrappers.
>> */
>>
>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>> ---
>>   lib/i915/gem_mman.c |  4 ++++
>>   lib/igt_fb.c        | 14 ++++++++------
>>   tests/kms_ccs.c     | 14 ++++++++------
>>   tests/kms_psr.c     |  8 ++++----
>>   4 files changed, 24 insertions(+), 16 deletions(-)
>>
>> diff --git a/lib/i915/gem_mman.c b/lib/i915/gem_mman.c
>> index 3cf9a6bbdb31..084dbb3b3678 100644
>> --- a/lib/i915/gem_mman.c
>> +++ b/lib/i915/gem_mman.c
>> @@ -57,6 +57,8 @@ void *__gem_mmap__gtt(int fd, uint32_t handle, uint64_t size, unsigned prot)
>>          struct drm_i915_gem_mmap_gtt mmap_arg;
>>          void *ptr;
>>   
>> +       igt_assert(!(size & 4095));
> 
> Fwiw, it's probably best if we had some kind of IGT_BUG_ON() that caused
> a much more visible explosion than a mere test failure.
> 
> #define IGT_BUG_ON(cond) if (!(cond)) raise(SIGILL)

No idea if that would be worthwhile.

But anyway, it seems I have identified all the tests which do 
non-page-aligned mmap. So..

a) leave the assert in low level mmap helpers
b) move the assert to higher level wrappers
c) round up in wrappers is not an option due unmap
d) do nothing in igt

?

Regards,

Tvrtko
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [igt-dev] [RFT i-g-t] lib/i915: Assert mmap size alignment
  2019-03-05  7:55     ` Tvrtko Ursulin
@ 2019-03-05  8:38       ` Chris Wilson
  -1 siblings, 0 replies; 14+ messages in thread
From: Chris Wilson @ 2019-03-05  8:38 UTC (permalink / raw)
  To: Tvrtko Ursulin, igt-dev; +Cc: Intel-gfx

Quoting Tvrtko Ursulin (2019-03-05 07:55:43)
> 
> On 04/03/2019 14:45, Chris Wilson wrote:
> > Quoting Tvrtko Ursulin (2019-03-04 14:11:31)
> >> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> >>
> >> Fishing for fails...
> >>
> >> /*
> >> mmap(2) mandates size is page aligned so check this in our wrappers.
> >> */
> >>
> >> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> >> ---
> >>   lib/i915/gem_mman.c |  4 ++++
> >>   lib/igt_fb.c        | 14 ++++++++------
> >>   tests/kms_ccs.c     | 14 ++++++++------
> >>   tests/kms_psr.c     |  8 ++++----
> >>   4 files changed, 24 insertions(+), 16 deletions(-)
> >>
> >> diff --git a/lib/i915/gem_mman.c b/lib/i915/gem_mman.c
> >> index 3cf9a6bbdb31..084dbb3b3678 100644
> >> --- a/lib/i915/gem_mman.c
> >> +++ b/lib/i915/gem_mman.c
> >> @@ -57,6 +57,8 @@ void *__gem_mmap__gtt(int fd, uint32_t handle, uint64_t size, unsigned prot)
> >>          struct drm_i915_gem_mmap_gtt mmap_arg;
> >>          void *ptr;
> >>   
> >> +       igt_assert(!(size & 4095));
> > 
> > Fwiw, it's probably best if we had some kind of IGT_BUG_ON() that caused
> > a much more visible explosion than a mere test failure.
> > 
> > #define IGT_BUG_ON(cond) if (!(cond)) raise(SIGILL)
> 
> No idea if that would be worthwhile.
> 
> But anyway, it seems I have identified all the tests which do 
> non-page-aligned mmap. So..
> 
> a) leave the assert in low level mmap helpers
> b) move the assert to higher level wrappers
> c) round up in wrappers is not an option due unmap
> d) do nothing in igt

Imo leave the wrappers alone, fix the bugs in the callers, fume that it
didn't explode earlier.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [igt-dev] [RFT i-g-t] lib/i915: Assert mmap size alignment
@ 2019-03-05  8:38       ` Chris Wilson
  0 siblings, 0 replies; 14+ messages in thread
From: Chris Wilson @ 2019-03-05  8:38 UTC (permalink / raw)
  To: Tvrtko Ursulin, igt-dev; +Cc: Intel-gfx, Tvrtko Ursulin

Quoting Tvrtko Ursulin (2019-03-05 07:55:43)
> 
> On 04/03/2019 14:45, Chris Wilson wrote:
> > Quoting Tvrtko Ursulin (2019-03-04 14:11:31)
> >> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> >>
> >> Fishing for fails...
> >>
> >> /*
> >> mmap(2) mandates size is page aligned so check this in our wrappers.
> >> */
> >>
> >> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> >> ---
> >>   lib/i915/gem_mman.c |  4 ++++
> >>   lib/igt_fb.c        | 14 ++++++++------
> >>   tests/kms_ccs.c     | 14 ++++++++------
> >>   tests/kms_psr.c     |  8 ++++----
> >>   4 files changed, 24 insertions(+), 16 deletions(-)
> >>
> >> diff --git a/lib/i915/gem_mman.c b/lib/i915/gem_mman.c
> >> index 3cf9a6bbdb31..084dbb3b3678 100644
> >> --- a/lib/i915/gem_mman.c
> >> +++ b/lib/i915/gem_mman.c
> >> @@ -57,6 +57,8 @@ void *__gem_mmap__gtt(int fd, uint32_t handle, uint64_t size, unsigned prot)
> >>          struct drm_i915_gem_mmap_gtt mmap_arg;
> >>          void *ptr;
> >>   
> >> +       igt_assert(!(size & 4095));
> > 
> > Fwiw, it's probably best if we had some kind of IGT_BUG_ON() that caused
> > a much more visible explosion than a mere test failure.
> > 
> > #define IGT_BUG_ON(cond) if (!(cond)) raise(SIGILL)
> 
> No idea if that would be worthwhile.
> 
> But anyway, it seems I have identified all the tests which do 
> non-page-aligned mmap. So..
> 
> a) leave the assert in low level mmap helpers
> b) move the assert to higher level wrappers
> c) round up in wrappers is not an option due unmap
> d) do nothing in igt

Imo leave the wrappers alone, fix the bugs in the callers, fume that it
didn't explode earlier.
-Chris
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [igt-dev] [RFT i-g-t] lib/i915: Assert mmap size alignment
  2019-03-05  8:38       ` Chris Wilson
@ 2019-03-05  9:23         ` Tvrtko Ursulin
  -1 siblings, 0 replies; 14+ messages in thread
From: Tvrtko Ursulin @ 2019-03-05  9:23 UTC (permalink / raw)
  To: Chris Wilson, igt-dev; +Cc: Intel-gfx


On 05/03/2019 08:38, Chris Wilson wrote:
> Quoting Tvrtko Ursulin (2019-03-05 07:55:43)
>>
>> On 04/03/2019 14:45, Chris Wilson wrote:
>>> Quoting Tvrtko Ursulin (2019-03-04 14:11:31)
>>>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>>>
>>>> Fishing for fails...
>>>>
>>>> /*
>>>> mmap(2) mandates size is page aligned so check this in our wrappers.
>>>> */
>>>>
>>>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>>> ---
>>>>    lib/i915/gem_mman.c |  4 ++++
>>>>    lib/igt_fb.c        | 14 ++++++++------
>>>>    tests/kms_ccs.c     | 14 ++++++++------
>>>>    tests/kms_psr.c     |  8 ++++----
>>>>    4 files changed, 24 insertions(+), 16 deletions(-)
>>>>
>>>> diff --git a/lib/i915/gem_mman.c b/lib/i915/gem_mman.c
>>>> index 3cf9a6bbdb31..084dbb3b3678 100644
>>>> --- a/lib/i915/gem_mman.c
>>>> +++ b/lib/i915/gem_mman.c
>>>> @@ -57,6 +57,8 @@ void *__gem_mmap__gtt(int fd, uint32_t handle, uint64_t size, unsigned prot)
>>>>           struct drm_i915_gem_mmap_gtt mmap_arg;
>>>>           void *ptr;
>>>>    
>>>> +       igt_assert(!(size & 4095));
>>>
>>> Fwiw, it's probably best if we had some kind of IGT_BUG_ON() that caused
>>> a much more visible explosion than a mere test failure.
>>>
>>> #define IGT_BUG_ON(cond) if (!(cond)) raise(SIGILL)
>>
>> No idea if that would be worthwhile.
>>
>> But anyway, it seems I have identified all the tests which do
>> non-page-aligned mmap. So..
>>
>> a) leave the assert in low level mmap helpers
>> b) move the assert to higher level wrappers
>> c) round up in wrappers is not an option due unmap
>> d) do nothing in igt
> 
> Imo leave the wrappers alone, fix the bugs in the callers, fume that it
> didn't explode earlier.

Not even a small igt_warn or tiny igt_debug in the wrappers? :)

Regards,

Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [igt-dev] [RFT i-g-t] lib/i915: Assert mmap size alignment
@ 2019-03-05  9:23         ` Tvrtko Ursulin
  0 siblings, 0 replies; 14+ messages in thread
From: Tvrtko Ursulin @ 2019-03-05  9:23 UTC (permalink / raw)
  To: Chris Wilson, igt-dev; +Cc: Intel-gfx, Tvrtko Ursulin


On 05/03/2019 08:38, Chris Wilson wrote:
> Quoting Tvrtko Ursulin (2019-03-05 07:55:43)
>>
>> On 04/03/2019 14:45, Chris Wilson wrote:
>>> Quoting Tvrtko Ursulin (2019-03-04 14:11:31)
>>>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>>>
>>>> Fishing for fails...
>>>>
>>>> /*
>>>> mmap(2) mandates size is page aligned so check this in our wrappers.
>>>> */
>>>>
>>>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>>> ---
>>>>    lib/i915/gem_mman.c |  4 ++++
>>>>    lib/igt_fb.c        | 14 ++++++++------
>>>>    tests/kms_ccs.c     | 14 ++++++++------
>>>>    tests/kms_psr.c     |  8 ++++----
>>>>    4 files changed, 24 insertions(+), 16 deletions(-)
>>>>
>>>> diff --git a/lib/i915/gem_mman.c b/lib/i915/gem_mman.c
>>>> index 3cf9a6bbdb31..084dbb3b3678 100644
>>>> --- a/lib/i915/gem_mman.c
>>>> +++ b/lib/i915/gem_mman.c
>>>> @@ -57,6 +57,8 @@ void *__gem_mmap__gtt(int fd, uint32_t handle, uint64_t size, unsigned prot)
>>>>           struct drm_i915_gem_mmap_gtt mmap_arg;
>>>>           void *ptr;
>>>>    
>>>> +       igt_assert(!(size & 4095));
>>>
>>> Fwiw, it's probably best if we had some kind of IGT_BUG_ON() that caused
>>> a much more visible explosion than a mere test failure.
>>>
>>> #define IGT_BUG_ON(cond) if (!(cond)) raise(SIGILL)
>>
>> No idea if that would be worthwhile.
>>
>> But anyway, it seems I have identified all the tests which do
>> non-page-aligned mmap. So..
>>
>> a) leave the assert in low level mmap helpers
>> b) move the assert to higher level wrappers
>> c) round up in wrappers is not an option due unmap
>> d) do nothing in igt
> 
> Imo leave the wrappers alone, fix the bugs in the callers, fume that it
> didn't explode earlier.

Not even a small igt_warn or tiny igt_debug in the wrappers? :)

Regards,

Tvrtko
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [igt-dev] [RFT i-g-t] lib/i915: Assert mmap size alignment
  2019-03-05  9:23         ` Tvrtko Ursulin
@ 2019-03-05  9:30           ` Chris Wilson
  -1 siblings, 0 replies; 14+ messages in thread
From: Chris Wilson @ 2019-03-05  9:30 UTC (permalink / raw)
  To: Tvrtko Ursulin, igt-dev; +Cc: Intel-gfx

Quoting Tvrtko Ursulin (2019-03-05 09:23:02)
> 
> On 05/03/2019 08:38, Chris Wilson wrote:
> > Quoting Tvrtko Ursulin (2019-03-05 07:55:43)
> >>
> >> On 04/03/2019 14:45, Chris Wilson wrote:
> >>> Quoting Tvrtko Ursulin (2019-03-04 14:11:31)
> >>>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> >>>>
> >>>> Fishing for fails...
> >>>>
> >>>> /*
> >>>> mmap(2) mandates size is page aligned so check this in our wrappers.
> >>>> */
> >>>>
> >>>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> >>>> ---
> >>>>    lib/i915/gem_mman.c |  4 ++++
> >>>>    lib/igt_fb.c        | 14 ++++++++------
> >>>>    tests/kms_ccs.c     | 14 ++++++++------
> >>>>    tests/kms_psr.c     |  8 ++++----
> >>>>    4 files changed, 24 insertions(+), 16 deletions(-)
> >>>>
> >>>> diff --git a/lib/i915/gem_mman.c b/lib/i915/gem_mman.c
> >>>> index 3cf9a6bbdb31..084dbb3b3678 100644
> >>>> --- a/lib/i915/gem_mman.c
> >>>> +++ b/lib/i915/gem_mman.c
> >>>> @@ -57,6 +57,8 @@ void *__gem_mmap__gtt(int fd, uint32_t handle, uint64_t size, unsigned prot)
> >>>>           struct drm_i915_gem_mmap_gtt mmap_arg;
> >>>>           void *ptr;
> >>>>    
> >>>> +       igt_assert(!(size & 4095));
> >>>
> >>> Fwiw, it's probably best if we had some kind of IGT_BUG_ON() that caused
> >>> a much more visible explosion than a mere test failure.
> >>>
> >>> #define IGT_BUG_ON(cond) if (!(cond)) raise(SIGILL)
> >>
> >> No idea if that would be worthwhile.
> >>
> >> But anyway, it seems I have identified all the tests which do
> >> non-page-aligned mmap. So..
> >>
> >> a) leave the assert in low level mmap helpers
> >> b) move the assert to higher level wrappers
> >> c) round up in wrappers is not an option due unmap
> >> d) do nothing in igt
> > 
> > Imo leave the wrappers alone, fix the bugs in the callers, fume that it
> > didn't explode earlier.
> 
> Not even a small igt_warn or tiny igt_debug in the wrappers? :)

Not particularly, if it didn't impact the result, at worst it's a small
mysterious leak (which should be catchable by valgrind).
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [igt-dev] [RFT i-g-t] lib/i915: Assert mmap size alignment
@ 2019-03-05  9:30           ` Chris Wilson
  0 siblings, 0 replies; 14+ messages in thread
From: Chris Wilson @ 2019-03-05  9:30 UTC (permalink / raw)
  To: Tvrtko Ursulin, igt-dev; +Cc: Intel-gfx, Tvrtko Ursulin

Quoting Tvrtko Ursulin (2019-03-05 09:23:02)
> 
> On 05/03/2019 08:38, Chris Wilson wrote:
> > Quoting Tvrtko Ursulin (2019-03-05 07:55:43)
> >>
> >> On 04/03/2019 14:45, Chris Wilson wrote:
> >>> Quoting Tvrtko Ursulin (2019-03-04 14:11:31)
> >>>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> >>>>
> >>>> Fishing for fails...
> >>>>
> >>>> /*
> >>>> mmap(2) mandates size is page aligned so check this in our wrappers.
> >>>> */
> >>>>
> >>>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> >>>> ---
> >>>>    lib/i915/gem_mman.c |  4 ++++
> >>>>    lib/igt_fb.c        | 14 ++++++++------
> >>>>    tests/kms_ccs.c     | 14 ++++++++------
> >>>>    tests/kms_psr.c     |  8 ++++----
> >>>>    4 files changed, 24 insertions(+), 16 deletions(-)
> >>>>
> >>>> diff --git a/lib/i915/gem_mman.c b/lib/i915/gem_mman.c
> >>>> index 3cf9a6bbdb31..084dbb3b3678 100644
> >>>> --- a/lib/i915/gem_mman.c
> >>>> +++ b/lib/i915/gem_mman.c
> >>>> @@ -57,6 +57,8 @@ void *__gem_mmap__gtt(int fd, uint32_t handle, uint64_t size, unsigned prot)
> >>>>           struct drm_i915_gem_mmap_gtt mmap_arg;
> >>>>           void *ptr;
> >>>>    
> >>>> +       igt_assert(!(size & 4095));
> >>>
> >>> Fwiw, it's probably best if we had some kind of IGT_BUG_ON() that caused
> >>> a much more visible explosion than a mere test failure.
> >>>
> >>> #define IGT_BUG_ON(cond) if (!(cond)) raise(SIGILL)
> >>
> >> No idea if that would be worthwhile.
> >>
> >> But anyway, it seems I have identified all the tests which do
> >> non-page-aligned mmap. So..
> >>
> >> a) leave the assert in low level mmap helpers
> >> b) move the assert to higher level wrappers
> >> c) round up in wrappers is not an option due unmap
> >> d) do nothing in igt
> > 
> > Imo leave the wrappers alone, fix the bugs in the callers, fume that it
> > didn't explode earlier.
> 
> Not even a small igt_warn or tiny igt_debug in the wrappers? :)

Not particularly, if it didn't impact the result, at worst it's a small
mysterious leak (which should be catchable by valgrind).
-Chris
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2019-03-05  9:30 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-03-04 14:11 [RFT i-g-t] lib/i915: Assert mmap size alignment Tvrtko Ursulin
2019-03-04 14:11 ` [igt-dev] " Tvrtko Ursulin
2019-03-04 14:41 ` [igt-dev] ✓ Fi.CI.BAT: success for lib/i915: Assert mmap size alignment (rev4) Patchwork
2019-03-04 14:45 ` [igt-dev] [RFT i-g-t] lib/i915: Assert mmap size alignment Chris Wilson
2019-03-04 14:45   ` Chris Wilson
2019-03-05  7:55   ` Tvrtko Ursulin
2019-03-05  7:55     ` Tvrtko Ursulin
2019-03-05  8:38     ` Chris Wilson
2019-03-05  8:38       ` Chris Wilson
2019-03-05  9:23       ` Tvrtko Ursulin
2019-03-05  9:23         ` Tvrtko Ursulin
2019-03-05  9:30         ` Chris Wilson
2019-03-05  9:30           ` Chris Wilson
2019-03-04 18:33 ` [igt-dev] ✓ Fi.CI.IGT: success for lib/i915: Assert mmap size alignment (rev4) Patchwork

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.