On Mon, May 12, 2014 at 08:46:24PM +0300, ville.syrjala@xxxxxxxxxxxxxxx wrote:
> From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
>
> The kernel full ppgtt support has a bug where it can drop a pinned
> fence to the floor, hence we leak the pin_count as the subsequent
> fence unpin becomes a nop. We can trigger it easily by unbinding a
> buffer from a ppgtt address space while the buffer is simultaneosly
> being used for scanout.
>
> Monitor i915_gem_fence_regs to detect the leak while performing
> the required set of operations in a loop.
do we really need that part? I've hoped just leaking enough fences until
the kernel blows up should be sufficient.
Generally I'm vary of using debugfs in new tests a bit since we
essentially then lock down that debugfs file as abi. And since no one
easily notices when a test stops catching a bug that's usually bad. Hence
also why I want basic testcases for debugfs infrastructure to make sure it
keeps on working.
-Daniel
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> ---
> tests/Makefile.sources | 1 +
> tests/kms_fence_pin_leak.c | 224 +++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 225 insertions(+)
> create mode 100644 tests/kms_fence_pin_leak.c
>
> diff --git a/tests/Makefile.sources b/tests/Makefile.sources
> index 5d5dc46..393c4a2 100644
> --- a/tests/Makefile.sources
> +++ b/tests/Makefile.sources
> @@ -60,6 +60,7 @@ TESTS_progs_M = \
> kms_addfb \
> kms_cursor_crc \
> kms_fbc_crc \
> + kms_fence_pin_leak \
> kms_flip \
> kms_flip_tiling \
> kms_pipe_crc_basic \
> diff --git a/tests/kms_fence_pin_leak.c b/tests/kms_fence_pin_leak.c
> new file mode 100644
> index 0000000..f299212
> --- /dev/null
> +++ b/tests/kms_fence_pin_leak.c
> @@ -0,0 +1,224 @@
> +/*
> + * Copyright © 2014 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.
> + *
> + */
> +
> +#include <errno.h>
> +#include <limits.h>
> +#include <stdbool.h>
> +#include <stdio.h>
> +#include <string.h>
> +
> +#include "drmtest.h"
> +#include "igt_debugfs.h"
> +#include "igt_kms.h"
> +#include "ioctl_wrappers.h"
> +#include "intel_chipset.h"
> +
> +typedef struct {
> + int drm_fd;
> + uint32_t devid;
> + drm_intel_bufmgr *bufmgr;
> + igt_display_t display;
> +} data_t;
> +
> +static void exec_nop(data_t *data, uint32_t handle, drm_intel_context *context)
> +{
> + drm_intel_bo *dst;
> + struct intel_batchbuffer *batch;
> +
> + dst = gem_handle_to_libdrm_bo(data->bufmgr, data->drm_fd, "", handle);
> + igt_assert(dst);
> +
> + batch = intel_batchbuffer_alloc(data->bufmgr, data->devid);
> + igt_assert(batch);
> +
> + /* add the reloc to make sure the kernel will think we write to dst */
> + BEGIN_BATCH(4);
> + OUT_BATCH(MI_BATCH_BUFFER_END);
> + OUT_BATCH(MI_NOOP);
> + OUT_RELOC(dst, I915_GEM_DOMAIN_RENDER, I915_GEM_DOMAIN_RENDER, 0);
> + OUT_BATCH(MI_NOOP);
> + ADVANCE_BATCH();
> +
> + intel_batchbuffer_flush_with_context(batch, context);
> + intel_batchbuffer_free(batch);
> +
> + drm_intel_bo_unreference(dst);
> +}
> +
> +static unsigned int total_fence_pin_count(void)
> +{
> + unsigned int pinned = 0;
> + char buf[128];
> + FILE *f;
> +
> + f = igt_debugfs_fopen("i915_gem_fence_regs", "r");
> + igt_assert(f);
> +
> + for (;;) {
> + unsigned int pin_count = 0;
> + char *ptr;
> +
> + ptr = fgets(buf, sizeof(buf), f);
> + if (!ptr)
> + break;
> +
> + if (sscanf(ptr, "Fence %*u, pin count = %u", &pin_count) == 1)
> + pinned += pin_count;
> + }
> +
> + fclose(f);
> +
> + return pinned;
> +}
> +
> +static bool run_single_test(data_t *data, enum pipe pipe, igt_output_t *output)
> +{
> + igt_display_t *display = &data->display;
> + drmModeModeInfo *mode;
> + igt_plane_t *primary;
> + unsigned int pin_count;
> + struct igt_fb fb[2];
> + int i;
> +
> + igt_output_set_pipe(output, pipe);
> + igt_display_commit(display);
> +
> + if (!output->valid) {
> + igt_output_set_pipe(output, PIPE_ANY);
> + igt_display_commit(display);
> + return false;
> + }
> +
> + mode = igt_output_get_mode(output);
> + primary = igt_output_get_plane(output, IGT_PLANE_PRIMARY);
> +
> + igt_create_color_fb(data->drm_fd, mode->hdisplay, mode->vdisplay,
> + DRM_FORMAT_XRGB8888,
> + true, /* need a fence so must be tiled */
> + 0.0, 0.0, 0.0,
> + &fb[0]);
> + igt_create_color_fb(data->drm_fd, mode->hdisplay, mode->vdisplay,
> + DRM_FORMAT_XRGB8888,
> + true, /* need a fence so must be tiled */
> + 0.0, 0.0, 0.0,
> + &fb[1]);
> +
> + igt_plane_set_fb(primary, &fb[0]);
> + igt_display_commit(display);
> +
> + /* sample the fence pin_counts now that we have one scanout buffer pinned */
> + pin_count = total_fence_pin_count();
> +
> + for (i = 0; i < 64; i++) {
> + drm_intel_context *ctx;
> +
> + /*
> + * Link fb.gem_handle to the ppgtt vm of ctx so that the context
> + * destruction will unbind the obj from the ppgtt vm in question.
> + */
> + ctx = drm_intel_gem_context_create(data->bufmgr);
> + igt_assert(ctx);
> + exec_nop(data, fb[i&1].gem_handle, ctx);
> + drm_intel_gem_context_destroy(ctx);
> +
> + /* Force a context switch to make sure ctx gets destroyed for real. */
> + exec_nop(data, fb[i&1].gem_handle, NULL);
> +
> + gem_sync(data->drm_fd, fb[i&1].gem_handle);
> +
> + /*
> + * Pin the new buffer and unpin the old buffer from display. If
> + * the kernel is buggy the ppgtt unbind will have dropped the
> + * fence for the old buffer, and now the display code will try
> + * to unpin only to find no fence there. So the pin_count will leak.
> + */
> + igt_plane_set_fb(primary, &fb[!(i&1)]);
> + igt_display_commit(display);
> +
> + /*
> + * We always have exactly one scanout buffer, so the fence pin_count
> + * should remain static. Fail the test if we detect a leak.
> + */
> + igt_assert(pin_count == total_fence_pin_count());
> +
> + printf(".");
> + fflush(stdout);
> + }
> +
> + igt_plane_set_fb(primary, NULL);
> + igt_output_set_pipe(output, PIPE_ANY);
> + igt_display_commit(display);
> +
> + igt_remove_fb(data->drm_fd, &fb[1]);
> + igt_remove_fb(data->drm_fd, &fb[0]);
> +
> + printf("\n");
> +
> + return true;
> +}
> +
> +static void run_test(data_t *data)
> +{
> + igt_display_t *display = &data->display;
> + igt_output_t *output;
> + enum pipe p;
> +
> + for_each_connected_output(display, output) {
> + for (p = 0; p < igt_display_get_n_pipes(display); p++) {
> + if (run_single_test(data, p, output))
> + return; /* one time ought to be enough */
> + }
> + }
> +
> + igt_skip("no valid crtc/connector combinations found\n");
> +}
> +
> +igt_simple_main
> +{
> + drm_intel_context *ctx;
> + data_t data = {};
> +
> + igt_skip_on_simulation();
> +
> + data.drm_fd = drm_open_any();
> +
> + data.devid = intel_get_drm_devid(data.drm_fd);
> +
> + igt_set_vt_graphics_mode();
> +
> + data.bufmgr = drm_intel_bufmgr_gem_init(data.drm_fd, 4096);
> + igt_assert(data.bufmgr);
> + drm_intel_bufmgr_gem_enable_reuse(data.bufmgr);
> +
> + igt_display_init(&data.display, data.drm_fd);
> +
> + ctx = drm_intel_gem_context_create(data.bufmgr);
> + igt_require(ctx);
> + drm_intel_gem_context_destroy(ctx);
> +
> + run_test(&data);
> +
> + drm_intel_bufmgr_destroy(data.bufmgr);
> + igt_display_fini(&data.display);
> +}
> --
> 1.8.3.2
>
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
