[PATCH i-g-t 0/2] kms_flip: Test for page flip crtc->fb clobber bug

[PATCH i-g-t 0/2] kms_flip: Test for page flip crtc->fb clobber bug

[ville.syrjala]

This new test 'flip-vs-bad-tiling' will trigger the pin_count underflow
BUG in the kernel due to page flip clobbering the crtc->fb pointer when
queue_flip fails. With the kernel fix applied the test completes
succesfully.

I also included a small fix for a use-after-free bug I spotted.

[PATCH i-g-t 1/2] kms_flip: Add flip-vs-bad-tiling test

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

flip-vs-bad-tiling tests that page flipping to a Y-tiled buffer returns
an error correctly, rather than triggering kernel BUG for instance.

Create a third fb for this purpose. After the fb has been created,
change its tiling mode to Y. When performing a flip, target this
Y-tiled fb and make sure we get the expected error value.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 tests/kms_flip.c | 36 ++++++++++++++++++++++++++++++++----
 1 file changed, 32 insertions(+), 4 deletions(-)

diff --git a/tests/kms_flip.c b/tests/kms_flip.c
index 9571dbd..0da40d3 100644
--- a/tests/kms_flip.c
+++ b/tests/kms_flip.c
@@ -58,6 +58,7 @@
 #define TEST_RMFB		(1 << 13)
 #define TEST_HANG		(1 << 14)
 #define TEST_NOEVENT		(1 << 15)
+#define TEST_FB_BAD_TILING	(1 << 16)
 
 #define EVENT_FLIP		(1 << 0)
 #define EVENT_VBLANK		(1 << 1)
@@ -119,9 +120,9 @@ struct test_output {
 	unsigned int current_fb_id;
 	unsigned int fb_width;
 	unsigned int fb_height;
-	unsigned int fb_ids[2];
+	unsigned int fb_ids[3];
 	int bpp, depth;
-	struct kmstest_fb fb_info[2];
+	struct kmstest_fb fb_info[3];
 
 	struct event_state flip_state;
 	struct event_state vblank_state;
@@ -496,6 +497,20 @@ static void recreate_fb(struct test_output *o)
 	o->fb_info[o->current_fb_id].fb_id = new_fb_id;
 }
 
+static void set_y_tiling(struct test_output *o, int fb_idx)
+{
+	drmModeFBPtr r;
+	struct kmstest_fb *fb_info = &o->fb_info[fb_idx];
+
+	/* Call rmfb/getfb/addfb to ensure those don't introduce stalls */
+	r = drmModeGetFB(drm_fd, fb_info->fb_id);
+	assert(r);
+	gem_set_tiling(drm_fd, r->handle, I915_TILING_Y, fb_info->stride);
+	gem_close(drm_fd, r->handle);
+	drmFree(r);
+}
+
+
 static int exec_nop(int fd, uint32_t handle)
 {
 	struct drm_i915_gem_execbuffer2 execbuf;
@@ -600,6 +615,9 @@ static unsigned int run_test_step(struct test_output *o)
 		recreate_fb(o);
 	new_fb_id = o->fb_ids[o->current_fb_id];
 
+	if (o->flags & TEST_FB_BAD_TILING)
+		new_fb_id = o->fb_ids[2];
+
 	if ((o->flags & TEST_VBLANK_EXPIRED_SEQ) &&
 	    !(o->pending_events & EVENT_VBLANK) && o->flip_state.count > 0) {
 		struct vblank_reply reply;
@@ -619,6 +637,9 @@ static unsigned int run_test_step(struct test_output *o)
 	if (do_flip && (o->flags & TEST_EINVAL) && o->flip_state.count > 0)
 		assert(do_page_flip(o, new_fb_id, true) == expected_einval);
 
+	if (o->flags & TEST_FB_BAD_TILING)
+		new_fb_id = o->fb_ids[o->current_fb_id];
+
 	if (do_vblank && (o->flags & TEST_EINVAL) && o->vblank_state.count > 0)
 		assert(do_wait_for_vblank(o, o->pipe, target_seq, &vbl_reply)
 		       == -EINVAL);
@@ -699,7 +720,7 @@ static unsigned int run_test_step(struct test_output *o)
 		assert(do_wait_for_vblank(o, o->pipe, target_seq, &vbl_reply)
 		       == -EINVAL);
 
-	if (do_flip && (o->flags & TEST_EINVAL))
+	if (do_flip && (o->flags & TEST_EINVAL) && !(o->flags & TEST_FB_BAD_TILING))
 		assert(do_page_flip(o, new_fb_id, true) == expected_einval);
 
 	if (do_flip && (o->flags & TEST_HANG)) {
@@ -971,12 +992,17 @@ static void run_test_on_crtc(struct test_output *o, int crtc, int duration)
 	o->fb_ids[1] = kmstest_create_fb(drm_fd, o->fb_width, o->fb_height,
 					 o->bpp, o->depth, false, &o->fb_info[1],
 					 paint_flip_mode, (void *)true);
+	o->fb_ids[2] = kmstest_create_fb(drm_fd, o->fb_width, o->fb_height,
+					 o->bpp, o->depth, true, &o->fb_info[2],
+					 paint_flip_mode, (void *)true);
 
-	if (!o->fb_ids[0] || !o->fb_ids[1]) {
+	if (!o->fb_ids[0] || !o->fb_ids[1] || !o->fb_ids[2]) {
 		fprintf(stderr, "failed to create fbs\n");
 		exit(3);
 	}
 
+	set_y_tiling(o, 2);
+
 	kmstest_dump_mode(&o->mode);
 	if (drmModeSetCrtc(drm_fd, o->crtc, o->fb_ids[0], 0, 0,
 			   &o->id, 1, &o->mode)) {
@@ -1014,6 +1040,7 @@ static void run_test_on_crtc(struct test_output *o, int crtc, int duration)
 	fprintf(stdout, "\n%s on crtc %d, connector %d: PASSED\n\n",
 		o->test_name, crtc, o->id);
 
+	kmstest_remove_fb(drm_fd, o->fb_ids[2]);
 	kmstest_remove_fb(drm_fd, o->fb_ids[1]);
 	kmstest_remove_fb(drm_fd, o->fb_ids[0]);
 
@@ -1111,6 +1138,7 @@ int main(int argc, char **argv)
 			TEST_CHECK_TS, "flip-vs-blocking-wf-vblank" },
 		{ 15, TEST_FLIP | TEST_MODESET | TEST_HANG | TEST_NOEVENT, "flip-vs-modeset-vs-hang" },
 		{ 15, TEST_FLIP | TEST_PAN | TEST_HANG, "flip-vs-panning-vs-hang" },
+		{ 1, TEST_FLIP | TEST_EINVAL | TEST_FB_BAD_TILING, "flip-vs-bad-tiling" },
 	};
 	int i;
 
-- 
1.7.12.4

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

[PATCH i-g-t 2/2] kms_flip: Don't access freed data

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 tests/kms_flip.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/kms_flip.c b/tests/kms_flip.c
index 0da40d3..902ac31 100644
--- a/tests/kms_flip.c
+++ b/tests/kms_flip.c
@@ -489,8 +489,8 @@ static void recreate_fb(struct test_output *o)
 			       o->bpp, fb_info->stride,
 			       r->handle, &new_fb_id));
 
-	drmFree(r);
 	gem_close(drm_fd, r->handle);
+	drmFree(r);
 	do_or_die(drmModeRmFB(drm_fd, fb_info->fb_id));
 
 	o->fb_ids[o->current_fb_id] = new_fb_id;
-- 
1.7.12.4

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

Re: [PATCH i-g-t 2/2] kms_flip: Don't access freed data

[Daniel Vetter]

On Mon, Mar 04, 2013 at 03:34:07PM +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

All four kms_flip patches applied, thanks a lot.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch