[Intel-gfx] [PATCH 1/3] igt/gem_stolen: Verifying extended gem_create ioctl

Tvrtko Ursulin tvrtko.ursulin at linux.intel.com
Wed Jul 22 09:00:08 PDT 2015


Hi,

On 07/22/2015 02:45 PM, ankitprasad.r.sharma at intel.com wrote:
> From: Ankitprasad Sharma <ankitprasad.r.sharma at intel.com>
>
> This patch adds the testcases for verifying the new extended
> gem_create ioctl. By means of this extended ioctl, memory
> placement of the GEM object can be specified, i.e. either
> shmem or stolen memory.
> These testcases include functional tests and interface tests for
> testing the gem_create ioctl call for stolen memory placement
>
> v2: Testing pread/pwrite functionality for stolen backed objects,
> added local struct for extended gem_create and gem_get_aperture,
> until headers catch up (Chris)
>
> v3: Removed get_aperture related functions, extended gem_pread
> to compare speeds for user pages with and without page faults,
> unexposed local_gem_create struct, changed gem_create_stolen
> usage (Chris)
>
> v4: Splitting patch to remove changes from gem_pread/gem_pwrite
> to another patch (Ankit)
>
> v5: Fixed Rebase conflicts (Ankit)
>      Added IGT_TEST_DESCRIPTION (Thomas Wood)
>
> v6: Added __gem_create_stolen for allowing error handling by User,
> updated gem_create_stolen to align with gem_create function, corrected
> fill_purge test (out of bound access), added testcase to validate
> allocating of more than 32 bit sized buffers (Tvrtko)
>
> Signed-off-by: Ankitprasad Sharma <ankitprasad.r.sharma at intel.com>
> ---
>   lib/ioctl_wrappers.c   |  74 ++++++++++
>   lib/ioctl_wrappers.h   |  13 ++
>   tests/Makefile.sources |   1 +
>   tests/gem_stolen.c     | 363 +++++++++++++++++++++++++++++++++++++++++++++++++
>   4 files changed, 451 insertions(+)
>   create mode 100644 tests/gem_stolen.c
>
> diff --git a/lib/ioctl_wrappers.c b/lib/ioctl_wrappers.c
> index a269d0f..da65de5 100644
> --- a/lib/ioctl_wrappers.c
> +++ b/lib/ioctl_wrappers.c
> @@ -389,6 +389,80 @@ void gem_sync(int fd, uint32_t handle)
>   		       I915_GEM_DOMAIN_GTT, I915_GEM_DOMAIN_GTT);
>   }
>
> +bool gem_create__has_stolen_support(int fd)
> +{
> +	static int has_stolen_support = -1;
> +	struct drm_i915_getparam gp;
> +	int val = -1;
> +
> +	if (has_stolen_support < 0) {
> +		memset(&gp, 0, sizeof(gp));
> +		gp.param = 36; /* CREATE_VERSION */
> +		gp.value = &val;
> +
> +		/* Do we have the extended gem_create_ioctl? */
> +		ioctl(fd, DRM_IOCTL_I915_GETPARAM, &gp);
> +		has_stolen_support = val >= 2;
> +	}
> +
> +	return has_stolen_support;
> +}
> +
> +struct local_i915_gem_create_v2 {
> +	uint64_t size;
> +	uint32_t handle;
> +	uint32_t pad;
> +#define I915_CREATE_PLACEMENT_STOLEN (1<<0)
> +	uint32_t flags;
> +};
> +
> +#define LOCAL_IOCTL_I915_GEM_CREATE       DRM_IOWR(DRM_COMMAND_BASE + DRM_I915_GEM_CREATE, struct local_i915_gem_create_v2)
> +uint32_t __gem_create_stolen(int fd, uint64_t size)
> +{
> +	struct local_i915_gem_create_v2 create;
> +	int ret;
> +
> +	memset(&create, 0, sizeof(create));
> +	create.handle = 0;
> +	create.size = size;
> +	create.flags = I915_CREATE_PLACEMENT_STOLEN;
> +	ret = drmIoctl(fd, LOCAL_IOCTL_I915_GEM_CREATE, &create);
> +
> +	if (ret < 0)
> +		return 0;
> +
> +	errno = 0;
> +	return create.handle;
> +}
> +
> +/**
> + * gem_create_stolen:
> + * @fd: open i915 drm file descriptor
> + * @size: desired size of the buffer
> + * @flags: desired placement i.e. stolen or shmem

There are no flags in the function prototype. :)

> + *
> + * This wraps the new GEM_CREATE ioctl, which allocates a
> + * new gem buffer object of @size and placement based on @flags.
> + *
> + * Returns: The file-private handle of the created buffer object
> + */
> +
> +uint32_t gem_create_stolen(int fd, uint64_t size)
> +{
> +	struct local_i915_gem_create_v2 create;
> +	int ret;
> +
> +	memset(&create, 0, sizeof(create));
> +	create.handle = 0;
> +	create.size = size;
> +	create.flags = I915_CREATE_PLACEMENT_STOLEN;
> +	do_ioctl(fd, LOCAL_IOCTL_I915_GEM_CREATE, &create);
> +	igt_assert(create.handle);
> +
> +	return create.handle;
> +}
> +
> +
>   uint32_t __gem_create(int fd, int size)
>   {
>   	struct drm_i915_gem_create create;
> diff --git a/lib/ioctl_wrappers.h b/lib/ioctl_wrappers.h
> index bc5d4bd..ff1584a 100644
> --- a/lib/ioctl_wrappers.h
> +++ b/lib/ioctl_wrappers.h
> @@ -56,6 +56,9 @@ void gem_read(int fd, uint32_t handle, uint64_t offset, void *buf, uint64_t leng
>   void gem_set_domain(int fd, uint32_t handle,
>   		    uint32_t read_domains, uint32_t write_domain);
>   void gem_sync(int fd, uint32_t handle);
> +bool gem_create__has_stolen_support(int fd);
> +uint32_t __gem_create_stolen(int fd, uint64_t size);
> +uint32_t gem_create_stolen(int fd, uint64_t size);
>   uint32_t __gem_create(int fd, int size);
>   uint32_t gem_create(int fd, uint64_t size);
>   void gem_execbuf(int fd, struct drm_i915_gem_execbuffer2 *execbuf);
> @@ -67,6 +70,16 @@ bool gem_mmap__has_wc(int fd);
>   void *gem_mmap__wc(int fd, uint32_t handle, uint64_t offset, uint64_t size, unsigned prot);
>
>   /**
> + * gem_require_stolen_support:
> + * @fd: open i915 drm file descriptor
> + *
> + * Test macro to query whether support for allocating objects from stolen
> + * memory is available. Automatically skips through igt_require() if not.
> + */
> +#define gem_require_stolen_support(fd) \
> +			igt_require(gem_create__has_stolen_support(fd))
> +
> +/**
>    * gem_require_mmap_wc:
>    * @fd: open i915 drm file descriptor
>    *
> diff --git a/tests/Makefile.sources b/tests/Makefile.sources
> index b9479cc..324cbb5 100644
> --- a/tests/Makefile.sources
> +++ b/tests/Makefile.sources
> @@ -55,6 +55,7 @@ TESTS_progs_M = \
>   	gem_reset_stats \
>   	gem_ringfill \
>   	gem_set_tiling_vs_blt \
> +	gem_stolen \
>   	gem_storedw_batches_loop \
>   	gem_streaming_writes \
>   	gem_tiled_blits \
> diff --git a/tests/gem_stolen.c b/tests/gem_stolen.c
> new file mode 100644
> index 0000000..d59e143
> --- /dev/null
> +++ b/tests/gem_stolen.c
> @@ -0,0 +1,363 @@
> +/*
> + * 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:
> + *    Ankitprasad Sharma <ankitprasad.r.sharma at intel.com>
> + *
> + */
> +
> +/** @file gem_create_stolen.c
> + *
> + * This is a test for the extended gem_create ioctl, that includes allocation
> + * of object from stolen memory.
> + *
> + * The goal is to simply ensure the basics work, and invalid input combinations
> + * are rejected.
> + */
> +
> +#include <stdlib.h>
> +#include <sys/ioctl.h>
> +#include <stdio.h>
> +#include <string.h>
> +#include <fcntl.h>
> +#include <inttypes.h>
> +#include <errno.h>
> +#include <sys/stat.h>
> +#include <sys/time.h>
> +#include <getopt.h>
> +
> +#include <drm.h>
> +
> +#include "ioctl_wrappers.h"
> +#include "intel_bufmgr.h"
> +#include "intel_batchbuffer.h"
> +#include "intel_io.h"
> +#include "intel_chipset.h"
> +#include "igt_aux.h"
> +#include "drmtest.h"
> +#include "drm.h"
> +#include "i915_drm.h"
> +
> +IGT_TEST_DESCRIPTION("This test verifies the exetended gem_create ioctl,"
> +		     " that includes allocation of obj from stolen region");
> +#define CLEAR(s) memset(&s, 0, sizeof(s))
> +#define SIZE 1024*1024
> +#define DWORD_SIZE 4
> +#define DATA 0xdead
> +#define LARGE_SIZE 8*1024*1024*1024
> +#define MAX_OBJECTS 100
> +
> +static struct drm_intel_bufmgr *bufmgr;
> +static struct intel_batchbuffer *batch;
> +
> +static void verify_copy_op(drm_intel_bo *src, drm_intel_bo *dest)
> +{
> +	uint32_t *virt, i, ret;
> +	/* Fill the src BO with dwords */
> +	ret = drm_intel_gem_bo_map_gtt(src);
> +	igt_assert(!ret);
> +
> +	virt = src->virtual;
> +	for (i = 0; i < SIZE/DWORD_SIZE; i++)
> +		virt[i] = i;
> +
> +	intel_copy_bo(batch, dest, src, SIZE);
> +
> +	ret = drm_intel_gem_bo_map_gtt(dest);
> +	igt_assert(!ret);
> +
> +	virt = dest->virtual;
> +	/* verify */
> +	for (i = 0; i < SIZE/DWORD_SIZE; i++)
> +		igt_assert(virt[i] == i);

You did not like suggestion to use igt_assert_eq?

> +
> +	drm_intel_bo_unmap(src);
> +	drm_intel_bo_unmap(dest);
> +}
> +
> +static void stolen_pwrite(int fd)
> +{
> +	drm_intel_bo *bo;
> +	uint32_t buf[SIZE/DWORD_SIZE];
> +	uint32_t handle = 0;
> +	uint32_t *virt;
> +	int i, ret = 0;
> +
> +	for (i = 0; i < SIZE/DWORD_SIZE; i ++)

Stray space remains. :)

> +		buf[i] = DATA;
> +
> +	gem_require_stolen_support(fd);
> +
> +	handle = gem_create_stolen(fd, SIZE);
> +
> +	gem_write(fd, handle, 0, buf, SIZE);
> +	bo = gem_handle_to_libdrm_bo(bufmgr, fd, "bo", handle);
> +
> +	ret = drm_intel_gem_bo_map_gtt(bo);
> +	igt_assert(!ret);
> +
> +	virt = bo->virtual;
> +
> +	for (i = 0; i < SIZE/DWORD_SIZE; i ++)
> +		igt_assert(virt[i] == DATA);
> +
> +	drm_intel_bo_unmap(bo);
> +	drm_intel_bo_unreference(bo);
> +	gem_close(fd, handle);
> +}
> +
> +static void stolen_pread(int fd)
> +{
> +	drm_intel_bo *bo;
> +	uint32_t buf[SIZE/DWORD_SIZE];
> +	uint32_t handle = 0;
> +	uint32_t *virt;
> +	int i, ret = 0;
> +
> +	CLEAR(buf);
> +
> +	gem_require_stolen_support(fd);
> +
> +	handle = gem_create_stolen(fd, SIZE);
> +
> +	bo = gem_handle_to_libdrm_bo(bufmgr, fd, "bo", handle);
> +
> +	ret = drm_intel_gem_bo_map_gtt(bo);
> +	igt_assert(!ret);
> +
> +	virt = bo->virtual;
> +
> +	for (i = 0; i < SIZE/DWORD_SIZE; i ++)
> +		virt[i] = DATA;
> +
> +	drm_intel_bo_unmap(bo);
> +	drm_intel_bo_unreference(bo);
> +
> +	gem_read(fd, handle, 0, buf, SIZE);
> +
> +	for (i = 0; i < SIZE/DWORD_SIZE; i ++)
> +		igt_assert(buf[i] == DATA);
> +
> +	gem_close(fd, handle);
> +}
> +
> +static void copy_test(int fd)
> +{
> +	drm_intel_bo *src, *dest;
> +	uint32_t src_handle = 0, dest_handle = 0;
> +	int ret = 0;
> +
> +	gem_require_stolen_support(fd);
> +
> +	src_handle = gem_create_stolen(fd, SIZE);
> +	dest_handle = gem_create_stolen(fd, SIZE);
> +
> +	src = gem_handle_to_libdrm_bo(bufmgr, fd, "src_bo", src_handle);
> +	dest = gem_handle_to_libdrm_bo(bufmgr, fd, "dst_bo", dest_handle);
> +
> +	igt_assert(src != NULL);
> +	igt_assert(dest != NULL);
> +
> +	verify_copy_op(src, dest);
> +
> +	drm_intel_bo_unreference(src);
> +	drm_intel_bo_unreference(dest);
> +	gem_close(fd, src_handle);
> +	gem_close(fd, dest_handle);
> +}
> +
> +static void verify_object_clear(int fd)
> +{
> +	drm_intel_bo *bo;
> +	uint32_t handle = 0;
> +	uint32_t *virt;
> +	int i, ret = 0;
> +
> +	gem_require_stolen_support(fd);
> +
> +	handle = gem_create_stolen(fd, SIZE);
> +
> +	bo = gem_handle_to_libdrm_bo(bufmgr, fd, "verify_bo", handle);
> +	igt_assert(bo != NULL);
> +
> +	ret = drm_intel_gem_bo_map_gtt(bo);
> +	igt_assert(ret == 0);
> +
> +	/* Verify if the BO is zeroed */
> +	virt = bo->virtual;
> +	for (i = 0; i < SIZE / DWORD_SIZE; i++)
> +		igt_assert(virt[i] == 0);
> +
> +	drm_intel_bo_unmap(bo);
> +	drm_intel_bo_unreference(bo);
> +	gem_close(fd, handle);
> +}
> +
> +static void stolen_large_obj_alloc(int fd)
> +{
> +	uint32_t handle = 0;
> +
> +	gem_require_stolen_support(fd);
> +	handle = __gem_create_stolen(fd, LARGE_SIZE);
> +	igt_assert(handle == 0);
> +}

I think we need one more subtest, which will overflow the 32-bit size by 
say one page. This would expose the bug in the kernel patch "[PATCH 2/4] 
drm/i915: Support for creating Stolen memory backed objects".

> +static void stolen_fill_purge_test(int fd)
> +{
> +	drm_intel_bo *bo;
> +	int mret = 0, ret = 0, obj_count = 0, i = 0;

mret and ret unused?

Regards,

Tvrtko


More information about the Intel-gfx mailing list