[Linaro-mm-sig] Re: [RFC PATCH v2] drm/test: add a test suite for GEM objects backed by shmem

On Wed, Nov 08, 2023 at 02:42:03PM +0100, Marco Pagani wrote:

This patch introduces an initial KUnit test suite for GEM objects backed by shmem buffers.

Suggested-by: Javier Martinez Canillas javierm@redhat.com Signed-off-by: Marco Pagani marpagan@redhat.com

v2:

• Improved description of test cases
• Cleaner error handling using KUnit actions
• Alphabetical order in Kconfig and Makefile

---

drivers/gpu/drm/Kconfig | 9 +- drivers/gpu/drm/tests/Makefile | 5 +- drivers/gpu/drm/tests/drm_gem_shmem_test.c | 381 +++++++++++++++++++++ 3 files changed, 389 insertions(+), 6 deletions(-) create mode 100644 drivers/gpu/drm/tests/drm_gem_shmem_test.c

diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig index 3eee8636f847..a2551c8c393a 100644 --- a/drivers/gpu/drm/Kconfig +++ b/drivers/gpu/drm/Kconfig @@ -76,14 +76,15 @@ config DRM_KUNIT_TEST tristate "KUnit tests for DRM" if !KUNIT_ALL_TESTS depends on DRM && KUNIT select PRIME_NUMBERS

• select DRM_BUDDY select DRM_DISPLAY_DP_HELPER select DRM_DISPLAY_HELPER

• select DRM_LIB_RANDOM
• select DRM_KMS_HELPER
• select DRM_BUDDY

• select DRM_EXEC select DRM_EXPORT_FOR_TESTS if m
• select DRM_GEM_SHMEM_HELPER
• select DRM_KMS_HELPER select DRM_KUNIT_TEST_HELPERS

• select DRM_EXEC

• select DRM_LIB_RANDOM default KUNIT_ALL_TESTS help This builds unit tests for DRM. This option is not useful for

diff --git a/drivers/gpu/drm/tests/Makefile b/drivers/gpu/drm/tests/Makefile index ba7baa622675..d6183b3d7688 100644 --- a/drivers/gpu/drm/tests/Makefile +++ b/drivers/gpu/drm/tests/Makefile @@ -9,15 +9,16 @@ obj-$(CONFIG_DRM_KUNIT_TEST) += \ drm_connector_test.o \ drm_damage_helper_test.o \ drm_dp_mst_helper_test.o \

• drm_exec_test.o \ drm_format_helper_test.o \ drm_format_test.o \ drm_framebuffer_test.o \
• drm_gem_shmem_test.o \ drm_managed_test.o \ drm_mm_test.o \ drm_modes_test.o \ drm_plane_helper_test.o \ drm_probe_helper_test.o \

• drm_rect_test.o \
• drm_exec_test.o

• drm_rect_test.o

Thanks for reordering the tests and symbols, but they should part of a preliminary patch.

CFLAGS_drm_mm_test.o := $(DISABLE_STRUCTLEAK_PLUGIN) diff --git a/drivers/gpu/drm/tests/drm_gem_shmem_test.c b/drivers/gpu/drm/tests/drm_gem_shmem_test.c new file mode 100644 index 000000000000..983380490673 --- /dev/null +++ b/drivers/gpu/drm/tests/drm_gem_shmem_test.c @@ -0,0 +1,381 @@ +// SPDX-License-Identifier: GPL-2.0 +/*

• • KUnit test suite for GEM objects backed by shmem buffers
• • Copyright (C) 2023 Red Hat, Inc.
• • Author: Marco Pagani marpagan@redhat.com
• */

+#include <linux/dma-buf.h> +#include <linux/iosys-map.h> +#include <linux/sizes.h>

+#include <kunit/test.h>

+#include <drm/drm_device.h> +#include <drm/drm_drv.h> +#include <drm/drm_gem.h> +#include <drm/drm_gem_shmem_helper.h> +#include <drm/drm_kunit_helpers.h>

+#define TEST_SIZE SZ_1M +#define TEST_BYTE 0xae

+struct fake_dev {

• struct drm_device drm_dev;
• struct device *dev;

AFAICS, you're not using the dev pointer anywhere. You could remove the fake_dev struct entirely and pass the drm_device pointer in test->priv

+};

+/*

• • Wrappers to avoid an explicit type casting when passing action
• • functions to kunit_add_action().
• */

+static void kfree_wrapper(void *p) +{

• kfree(p);

+}

+static void sg_free_table_wrapper(void *sgt) +{

• sg_free_table(sgt);

+}

+static void drm_gem_shmem_free_wrapper(void *shmem) +{

• drm_gem_shmem_free(shmem);

+}

I think you need to explicitly cast the pointer (or do a temporary assignment to the proper type) to avoid a compiler warning.

+/*

• • Test creating a shmem GEM object backed by shmem buffer. The test
• • case succeeds if the GEM object is successfully allocated with the
• • shmem file node and object functions attributes set, and the size
• • attribute is equal to the correct size.
• */

Thanks for all those comments, it's super helpful

+static void drm_gem_shmem_test_obj_create(struct kunit *test) +{

• struct fake_dev *fdev = test->priv;
• struct drm_gem_shmem_object *shmem;
• shmem = drm_gem_shmem_create(&fdev->drm_dev, TEST_SIZE);
• KUNIT_ASSERT_NOT_ERR_OR_NULL(test, shmem);
• KUNIT_EXPECT_EQ(test, shmem->base.size, TEST_SIZE);
• KUNIT_EXPECT_NOT_NULL(test, shmem->base.filp);
• KUNIT_EXPECT_NOT_NULL(test, shmem->base.funcs);
• drm_gem_shmem_free(shmem);

+}

+/*

• • Test creating a shmem GEM object from a scatter/gather table exported
• • via a DMA-BUF. The test case succeed if the GEM object is successfully
• • created with the shmem file node attribute equal to NULL and the sgt
• • attribute pointing to the scatter/gather table that has been imported.
• */

+static void drm_gem_shmem_test_obj_create_private(struct kunit *test) +{

• struct fake_dev *fdev = test->priv;
• struct drm_gem_shmem_object *shmem;
• struct drm_gem_object *gem_obj;
• struct dma_buf buf_mock;
• struct dma_buf_attachment attach_mock;
• struct sg_table *sgt;
• char *buf;
• int ret;
• /* Create a mock scatter/gather table */
• buf = kunit_kzalloc(test, TEST_SIZE, GFP_KERNEL);
• KUNIT_ASSERT_NOT_NULL(test, buf);
• sgt = kzalloc(sizeof(*sgt), GFP_KERNEL);
• KUNIT_ASSERT_NOT_NULL(test, sgt);
• ret = kunit_add_action_or_reset(test, kfree_wrapper, sgt);
• KUNIT_ASSERT_EQ(test, ret, 0);
• ret = sg_alloc_table(sgt, 1, GFP_KERNEL);
• KUNIT_ASSERT_EQ(test, ret, 0);
• ret = kunit_add_action_or_reset(test, sg_free_table_wrapper, sgt);
• KUNIT_ASSERT_EQ(test, ret, 0);
• sg_init_one(sgt->sgl, buf, TEST_SIZE);
• /* Init a mock DMA-BUF */
• buf_mock.size = TEST_SIZE;
• attach_mock.dmabuf = &buf_mock;
• gem_obj = drm_gem_shmem_prime_import_sg_table(&fdev->drm_dev, &attach_mock, sgt);
• KUNIT_ASSERT_NOT_ERR_OR_NULL(test, gem_obj);
• KUNIT_EXPECT_EQ(test, gem_obj->size, TEST_SIZE);
• KUNIT_EXPECT_NULL(test, gem_obj->filp);
• KUNIT_EXPECT_NOT_NULL(test, gem_obj->funcs);
• /* The scatter/gather table will be freed by drm_gem_shmem_free */
• kunit_remove_action(test, sg_free_table_wrapper, sgt);
• kunit_remove_action(test, kfree_wrapper, sgt);
• shmem = to_drm_gem_shmem_obj(gem_obj);
• KUNIT_EXPECT_PTR_EQ(test, shmem->sgt, sgt);
• drm_gem_shmem_free(shmem);

+}

+/*

• • Test pinning backing pages for a shmem GEM object. The test case
• • succeeds if a suitable number of backing pages are allocated, and
• • the pages table counter attribute is increased by one.
• */

+static void drm_gem_shmem_test_pin_pages(struct kunit *test) +{

• struct fake_dev *fdev = test->priv;
• struct drm_gem_shmem_object *shmem;
• int i, ret;
• shmem = drm_gem_shmem_create(&fdev->drm_dev, TEST_SIZE);
• KUNIT_ASSERT_NOT_ERR_OR_NULL(test, shmem);
• KUNIT_EXPECT_NULL(test, shmem->pages);
• KUNIT_EXPECT_EQ(test, shmem->pages_use_count, 0);
• ret = kunit_add_action_or_reset(test, drm_gem_shmem_free_wrapper, shmem);
• KUNIT_ASSERT_EQ(test, ret, 0);
• ret = drm_gem_shmem_pin(shmem);
• KUNIT_ASSERT_EQ(test, ret, 0);
• KUNIT_ASSERT_NOT_NULL(test, shmem->pages);
• KUNIT_EXPECT_EQ(test, shmem->pages_use_count, 1);
• for (i = 0; i < (shmem->base.size >> PAGE_SHIFT); i++)

• KUNIT_ASSERT_NOT_NULL(test, shmem->pages[i]);
  

• drm_gem_shmem_unpin(shmem);
• KUNIT_EXPECT_NULL(test, shmem->pages);
• KUNIT_EXPECT_EQ(test, shmem->pages_use_count, 0);

+}

+/*

• • Test creating a virtual mapping for a shmem GEM object. The test
• • case succeeds if the backing memory is mapped and the reference
• • counter for virtual mapping is increased by one. Moreover, the test
• • case writes and then reads a test pattern over the mapped memory.
• */

+static void drm_gem_shmem_test_vmap(struct kunit *test) +{

• struct fake_dev *fdev = test->priv;
• struct drm_gem_shmem_object *shmem;
• struct iosys_map map;
• int ret, i;
• shmem = drm_gem_shmem_create(&fdev->drm_dev, TEST_SIZE);
• KUNIT_ASSERT_NOT_ERR_OR_NULL(test, shmem);
• KUNIT_EXPECT_NULL(test, shmem->vaddr);
• KUNIT_EXPECT_EQ(test, shmem->vmap_use_count, 0);
• ret = kunit_add_action_or_reset(test, drm_gem_shmem_free_wrapper, shmem);
• KUNIT_ASSERT_EQ(test, ret, 0);
• ret = drm_gem_shmem_vmap(shmem, &map);
• KUNIT_ASSERT_EQ(test, ret, 0);
• KUNIT_ASSERT_NOT_NULL(test, shmem->vaddr);
• KUNIT_ASSERT_FALSE(test, iosys_map_is_null(&map));
• KUNIT_EXPECT_EQ(test, shmem->vmap_use_count, 1);
• iosys_map_memset(&map, 0, TEST_BYTE, TEST_SIZE);
• for (i = 0; i < TEST_SIZE; i++)

• KUNIT_EXPECT_EQ(test, iosys_map_rd(&map, i, u8), TEST_BYTE);
  

• drm_gem_shmem_vunmap(shmem, &map);
• KUNIT_EXPECT_NULL(test, shmem->vaddr);
• KUNIT_EXPECT_EQ(test, shmem->vmap_use_count, 0);

+}

+/*

• • Test exporting a scatter/gather table of pinned pages suitable for
• • PRIME usage from a shmem GEM object. The test case succeeds if a
• • scatter/gather table large enough to accommodate the backing memory
• • is successfully exported.
• */

+static void drm_gem_shmem_test_get_pages_sgt(struct kunit *test) +{

• struct fake_dev *fdev = test->priv;
• struct drm_gem_shmem_object *shmem;
• struct sg_table *sgt;
• struct scatterlist *sg;
• unsigned int si, len = 0;
• int ret;
• shmem = drm_gem_shmem_create(&fdev->drm_dev, TEST_SIZE);
• KUNIT_ASSERT_NOT_ERR_OR_NULL(test, shmem);
• ret = kunit_add_action_or_reset(test, drm_gem_shmem_free_wrapper, shmem);
• KUNIT_ASSERT_EQ(test, ret, 0);
• ret = drm_gem_shmem_pin(shmem);
• KUNIT_ASSERT_EQ(test, ret, 0);
• sgt = drm_gem_shmem_get_sg_table(shmem);
• KUNIT_ASSERT_NOT_ERR_OR_NULL(test, sgt);
• KUNIT_EXPECT_NULL(test, shmem->sgt);
• ret = kunit_add_action_or_reset(test, sg_free_table_wrapper, sgt);
• KUNIT_ASSERT_EQ(test, ret, 0);
• for_each_sgtable_sg(sgt, sg, si) {

• KUNIT_EXPECT_NOT_NULL(test, sg);
  

• len += sg->length;
  

• }
• KUNIT_EXPECT_GE(test, len, TEST_SIZE);

+}

+/*

• • Test pinning pages and exporting a scatter/gather table suitable for
• • driver usage from a shmem GEM object. The test case succeeds if the
• • backing pages are pinned and a scatter/gather table large enough to
• • accommodate the backing memory is successfully exported.
• */

+static void drm_gem_shmem_test_get_sg_table(struct kunit *test) +{

• struct fake_dev *fdev = test->priv;
• struct drm_gem_shmem_object *shmem;
• struct sg_table *sgt;
• struct scatterlist *sg;
• unsigned int si, len, ret = 0;
• shmem = drm_gem_shmem_create(&fdev->drm_dev, TEST_SIZE);
• KUNIT_ASSERT_NOT_ERR_OR_NULL(test, shmem);
• ret = kunit_add_action_or_reset(test, drm_gem_shmem_free_wrapper, shmem);
• KUNIT_ASSERT_EQ(test, ret, 0);
• /* The scatter/gather table will be freed by drm_gem_shmem_free */
• sgt = drm_gem_shmem_get_pages_sgt(shmem);
• KUNIT_ASSERT_NOT_ERR_OR_NULL(test, sgt);
• KUNIT_ASSERT_NOT_NULL(test, shmem->pages);
• KUNIT_EXPECT_EQ(test, shmem->pages_use_count, 1);
• KUNIT_EXPECT_PTR_EQ(test, sgt, shmem->sgt);
• for_each_sgtable_sg(sgt, sg, si) {

• KUNIT_EXPECT_NOT_NULL(test, sg);
  

• len += sg->length;
  

• }
• KUNIT_EXPECT_GE(test, len, TEST_SIZE);

+}

+/*

• • Test updating the madvise state of a shmem GEM object. The test
• • case checks that the function for setting madv updates it only if
• • its current value is greater or equal than zero and returns false
• • if it has a negative value.
• */

+static void drm_gem_shmem_test_madvise(struct kunit *test) +{

• struct fake_dev *fdev = test->priv;
• struct drm_gem_shmem_object *shmem;
• int ret;
• shmem = drm_gem_shmem_create(&fdev->drm_dev, TEST_SIZE);
• KUNIT_ASSERT_NOT_ERR_OR_NULL(test, shmem);
• KUNIT_ASSERT_EQ(test, shmem->madv, 0);
• ret = kunit_add_action_or_reset(test, drm_gem_shmem_free_wrapper, shmem);
• KUNIT_ASSERT_EQ(test, ret, 0);
• ret = drm_gem_shmem_madvise(shmem, 1);
• KUNIT_EXPECT_TRUE(test, ret);
• KUNIT_ASSERT_EQ(test, shmem->madv, 1);
• /* Set madv to a negative value */
• ret = drm_gem_shmem_madvise(shmem, -1);
• KUNIT_EXPECT_FALSE(test, ret);
• KUNIT_ASSERT_EQ(test, shmem->madv, -1);
• /* Check that madv cannot be set back to a positive value */
• ret = drm_gem_shmem_madvise(shmem, 0);
• KUNIT_EXPECT_FALSE(test, ret);
• KUNIT_ASSERT_EQ(test, shmem->madv, -1);

+}

+/*

• • Test purging a shmem GEM object. First, assert that a newly created
• • shmem GEM object is not purgeable. Then, set madvise to a positive
• • value and call drm_gem_shmem_get_pages_sgt() to pin and dma-map the
• • backing pages. Finally, assert that the shmem GEM object is now
• • purgeable and purge it.
• */

+static void drm_gem_shmem_test_purge(struct kunit *test) +{

• struct fake_dev *fdev = test->priv;
• struct drm_gem_shmem_object *shmem;
• struct sg_table *sgt;
• int ret;
• shmem = drm_gem_shmem_create(&fdev->drm_dev, TEST_SIZE);
• KUNIT_ASSERT_NOT_ERR_OR_NULL(test, shmem);
• ret = kunit_add_action_or_reset(test, drm_gem_shmem_free_wrapper, shmem);
• KUNIT_ASSERT_EQ(test, ret, 0);
• ret = drm_gem_shmem_is_purgeable(shmem);
• KUNIT_EXPECT_FALSE(test, ret);
• ret = drm_gem_shmem_madvise(shmem, 1);
• KUNIT_EXPECT_TRUE(test, ret);
• /* The scatter/gather table will be freed by drm_gem_shmem_free */
• sgt = drm_gem_shmem_get_pages_sgt(shmem);
• KUNIT_ASSERT_NOT_ERR_OR_NULL(test, sgt);
• ret = drm_gem_shmem_is_purgeable(shmem);
• KUNIT_EXPECT_TRUE(test, ret);
• drm_gem_shmem_purge(shmem);
• KUNIT_EXPECT_NULL(test, shmem->pages);
• KUNIT_EXPECT_NULL(test, shmem->sgt);
• KUNIT_EXPECT_EQ(test, shmem->madv, -1);

+}

+static int drm_gem_shmem_test_init(struct kunit *test) +{

• struct fake_dev *fdev;
• struct device *dev;
• /* Allocate a parent device */
• dev = drm_kunit_helper_alloc_device(test);
• KUNIT_ASSERT_NOT_ERR_OR_NULL(test, dev);
• /*

• * The DRM core will automatically initialize the GEM core and create
  

• * a DRM Memory Manager object which provides an address space pool
  

• * for GEM objects allocation.
  

• */
  

• fdev = drm_kunit_helper_alloc_drm_device(test, dev, struct fake_dev,

• 				 drm_dev, DRIVER_GEM);
  

• KUNIT_ASSERT_NOT_ERR_OR_NULL(test, fdev);
• fdev->dev = dev;
• test->priv = fdev;
• return 0;

+}

+static struct kunit_case drm_gem_shmem_test_cases[] = {

• KUNIT_CASE(drm_gem_shmem_test_obj_create),
• KUNIT_CASE(drm_gem_shmem_test_obj_create_private),
• KUNIT_CASE(drm_gem_shmem_test_pin_pages),
• KUNIT_CASE(drm_gem_shmem_test_vmap),
• KUNIT_CASE(drm_gem_shmem_test_get_pages_sgt),
• KUNIT_CASE(drm_gem_shmem_test_get_sg_table),
• KUNIT_CASE(drm_gem_shmem_test_madvise),
• KUNIT_CASE(drm_gem_shmem_test_purge),
• {}

+};

+static struct kunit_suite drm_gem_shmem_suite = {

• .name = "drm_gem_shmem",
• .init = drm_gem_shmem_test_init,
• .test_cases = drm_gem_shmem_test_cases

+};

+kunit_test_suite(drm_gem_shmem_suite);

Looks great otherwise, thanks! Maxime