Re: [Intel-gfx] [PATCH 1/2] drm/i915: Use CPU mapping for userspace dma-buf mmap()

From: Tiago Vignatti <tiago.vignatti@intel.com>
To: Chris Wilson <chris@chris-wilson.co.uk>,
	dri-devel@lists.freedesktop.org, daniel.vetter@ffwll.ch,
	intel-gfx@lists.freedesktop.org, daniel.thompson@linaro.org
Subject: Re: [Intel-gfx] [PATCH 1/2] drm/i915: Use CPU mapping for userspace dma-buf mmap()
Date: Tue, 04 Aug 2015 18:30:25 -0300	[thread overview]
Message-ID: <55C12EF1.7080701@intel.com> (raw)
In-Reply-To: <20150731210234.GA16824@nuc-i3427.alporthouse.com>

On 07/31/2015 06:02 PM, Chris Wilson wrote:
>
> The first problem is that llc does not guarrantee that the buffer is
> cache coherent with all aspects of the GPU. For scanout and similar
> writes need to be WC.
>
> if (obj->has_framebuffer_references) would at least catch where the fb
> is made before the mmap.
>
> Equally this buffer could then be shared with other devices and exposing
> a CPU mmap to userspace (and no flush/set-domain protocol) will result in
> corruption.

I've built an igt test to catch this corruption but it's not really 
falling there in my IvyBridge. If what you described is right (and so 
what I coded) then this test should write in the mapped buffer but not 
update the screen.

Any idea what's going on?

https://github.com/tiagovignatti/intel-gpu-tools/commit/3e130ac2b274f1a3f68855559c78cb72d0673ca2.patch


 From 3e130ac2b274f1a3f68855559c78cb72d0673ca2 Mon Sep 17 00:00:00 2001
From: Tiago Vignatti <tiago.vignatti@intel.com>
Date: Tue, 4 Aug 2015 13:38:09 -0300
Subject: [PATCH] tests: Add prime_crc for cache coherency

This program can be used to detect when the writes don't land in 
scanout, due
cache incoherency.

Run it like ./prime_crc --interactive-debug=crc

Signed-off-by: Tiago Vignatti <tiago.vignatti@intel.com>
---
  tests/.gitignore       |   1 +
  tests/Makefile.sources |   1 +
  tests/prime_crc.c      | 201 
+++++++++++++++++++++++++++++++++++++++++++++++++
  3 files changed, 203 insertions(+)
  create mode 100644 tests/prime_crc.c

diff --git a/tests/.gitignore b/tests/.gitignore
index 5bc4a58..96dbf57 100644
--- a/tests/.gitignore
+++ b/tests/.gitignore
@@ -160,6 +160,7 @@ pm_rc6_residency
  pm_rpm
  pm_rps
  pm_sseu
+prime_crc
  prime_nv_api
  prime_nv_pcopy
  prime_nv_test
diff --git a/tests/Makefile.sources b/tests/Makefile.sources
index 5b2072e..c05b5a7 100644
--- a/tests/Makefile.sources
+++ b/tests/Makefile.sources
@@ -90,6 +90,7 @@ TESTS_progs_M = \
  	pm_rps \
  	pm_rc6_residency \
  	pm_sseu \
+	prime_crc \
  	prime_mmap \
  	prime_self_import \
  	template \
diff --git a/tests/prime_crc.c b/tests/prime_crc.c
new file mode 100644
index 0000000..3474cc9
--- /dev/null
+++ b/tests/prime_crc.c
@@ -0,0 +1,201 @@
+/*
+ * Copyright © 2015 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:
+ *    Tiago Vignatti <tiago.vignatti at intel.com>
+ *
+ */
+
+/* This program can detect when the writes don't land in scanout, due cache
+ * incoherency. */
+
+#include "drmtest.h"
+#include "igt_debugfs.h"
+#include "igt_kms.h"
+
+#define MAX_CONNECTORS 32
+
+struct modeset_params {
+	uint32_t crtc_id;
+	uint32_t connector_id;
+	drmModeModeInfoPtr mode;
+};
+
+int drm_fd;
+drmModeResPtr drm_res;
+drmModeConnectorPtr drm_connectors[MAX_CONNECTORS];
+drm_intel_bufmgr *bufmgr;
+igt_pipe_crc_t *pipe_crc;
+
+struct modeset_params ms;
+
+static void find_modeset_params(void)
+{
+	int i;
+	uint32_t connector_id = 0, crtc_id;
+	drmModeModeInfoPtr mode = NULL;
+
+	for (i = 0; i < drm_res->count_connectors; i++) {
+		drmModeConnectorPtr c = drm_connectors[i];
+
+		if (c->count_modes) {
+			connector_id = c->connector_id;
+			mode = &c->modes[0];
+			break;
+		}
+	}
+	igt_require(connector_id);
+
+	crtc_id = drm_res->crtcs[0];
+	igt_assert(crtc_id);
+	igt_assert(mode);
+
+	ms.connector_id = connector_id;
+	ms.crtc_id = crtc_id;
+	ms.mode = mode;
+
+}
+
+#define BO_SIZE (16*1024)
+
+char pattern[] = {0xff, 0x00, 0x00, 0x00,
+	0x00, 0xff, 0x00, 0x00,
+	0x00, 0x00, 0xff, 0x00,
+	0x00, 0x00, 0x00, 0xff};
+
+static void mess_with_coherency(char *ptr)
+{
+	off_t i;
+
+	for (i = 0; i < BO_SIZE; i+=sizeof(pattern)) {
+		memcpy(ptr + i, pattern, sizeof(pattern));
+	}
+//	munmap(ptr, BO_SIZE);
+//	close(dma_buf_fd);
+}
+
+static char *dmabuf_mmap_framebuffer(struct igt_fb *fb)
+{
+	int dma_buf_fd;
+	char *ptr = NULL;
+
+	dma_buf_fd = prime_handle_to_fd(drm_fd, fb->gem_handle);
+	igt_assert(errno == 0);
+
+	ptr = mmap(NULL, BO_SIZE, PROT_READ | PROT_WRITE, MAP_SHARED, 
dma_buf_fd, 0);
+	igt_assert(ptr != MAP_FAILED);
+
+	return ptr;
+}
+
+static void get_method_crc(uint64_t tiling, igt_crc_t *crc, bool mess)
+{
+	struct igt_fb fb;
+	int rc;
+	char *ptr;
+
+	igt_create_fb(drm_fd, ms.mode->hdisplay, ms.mode->vdisplay,
+		      DRM_FORMAT_XRGB8888, tiling, &fb);
+
+	if (mess)
+		ptr = dmabuf_mmap_framebuffer(&fb);
+
+	rc = drmModeSetCrtc(drm_fd, ms.crtc_id, fb.fb_id, 0, 0,
+			    &ms.connector_id, 1, ms.mode);
+	igt_assert(rc == 0);
+
+	if (mess)
+		mess_with_coherency(ptr);
+
+	igt_pipe_crc_collect_crc(pipe_crc, crc);
+
+	kmstest_unset_all_crtcs(drm_fd, drm_res);
+	igt_remove_fb(drm_fd, &fb);
+}
+
+static void draw_method_subtest(uint64_t tiling)
+{
+	igt_crc_t reference_crc, crc;
+
+	kmstest_unset_all_crtcs(drm_fd, drm_res);
+
+	find_modeset_params();
+
+	get_method_crc(tiling, &reference_crc, false);
+	get_method_crc(tiling, &crc, true);
+
+	// XXX: IIUC if we mess up with the scanout device, through a dma-buf 
mmap'ed
+	// pointer, then both the reference crc and the messed up one should 
be equal
+	// because the latter wasn't flushed. That's the theory, but it's not 
what's
+	// happening and the following is not passing.
+	igt_assert_crc_equal(&reference_crc, &crc);
+}
+
+static void setup_environment(void)
+{
+	int i;
+
+	drm_fd = drm_open_any_master();
+	igt_require(drm_fd >= 0);
+
+	drm_res = drmModeGetResources(drm_fd);
+	igt_assert(drm_res->count_connectors <= MAX_CONNECTORS);
+
+	for (i = 0; i < drm_res->count_connectors; i++)
+		drm_connectors[i] = drmModeGetConnector(drm_fd,
+							drm_res->connectors[i]);
+
+	kmstest_set_vt_graphics_mode();
+
+	bufmgr = drm_intel_bufmgr_gem_init(drm_fd, 4096);
+	igt_assert(bufmgr);
+	drm_intel_bufmgr_gem_enable_reuse(bufmgr);
+
+	pipe_crc = igt_pipe_crc_new(0, INTEL_PIPE_CRC_SOURCE_AUTO);
+}
+
+static void teardown_environment(void)
+{
+	int i;
+
+	igt_pipe_crc_free(pipe_crc);
+
+	drm_intel_bufmgr_destroy(bufmgr);
+
+	for (i = 0; i < drm_res->count_connectors; i++)
+		drmModeFreeConnector(drm_connectors[i]);
+
+	drmModeFreeResources(drm_res);
+	close(drm_fd);
+}
+
+igt_main
+{
+	igt_fixture
+		setup_environment();
+
+	igt_subtest_f("draw-method-tiled")
+		draw_method_subtest(LOCAL_I915_FORMAT_MOD_X_TILED);
+
+	igt_fixture
+		teardown_environment();
+}
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
