On Tue, Aug 04, 2015 at 06:30:25PM -0300, Tiago Vignatti wrote:
> 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 at 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 at 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.
Nah they don't have to be equal since the problem isn't that nothing goes
out to memory where the display can see it, but usually only parts of it.
I.e. you need to change your test to
- draw black screen (it starts that way so nothing to do really), grab crtc
- draw white screen and make sure you flush correctly, don't bother with
crc (we can't test for inequality
because collisions are too easy)
- draw black screen again without flushing, grab crc
Then assert that your two crc will be inequal (which they shouldn't be
because some cachelines will still be stuck). Maybe also add a delay
somewhere so you can see the cacheline dirt pattern, it's very
characteristic.
-Daniel
> + 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();
> +}
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
