[igt-dev] [PATCH i-g-t 0/2] Add negative tests to VGEM

[igt-dev] [PATCH i-g-t 0/2] Add negative tests to VGEM

[Maíra Canal]

Currently, the VGEM tests don't cover any negative tests. So, by inspecting
the driver code, add tests that input invalid arguments and then, check if
the driver is returning the correct values.

In the first patch, update VGEM to use the UAPI header instead of having it
defined in the source code and in the second patch, introduce the negative
tests.

Best Regards,
- Maíra Canal

Maíra Canal (2):
  lib/igt_vgem: Use UAPI header
  tests/vgem_basic: Add negative tests

 lib/igt_vgem.c     | 36 +++++++-------------------
 lib/igt_vgem.h     |  4 +--
 tests/vgem_basic.c | 63 ++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 73 insertions(+), 30 deletions(-)

-- 
2.39.2

[igt-dev] [PATCH i-g-t 1/2] lib/igt_vgem: Use UAPI header

[Maíra Canal]

Currently, VGEM copies the UAPI header contents to source file. So,
delete the copy from the source file and use the UAPI header directly.

Signed-off-by: Maíra Canal <mcanal@igalia.com>
---
 lib/igt_vgem.c | 36 +++++++++---------------------------
 lib/igt_vgem.h |  4 +---
 2 files changed, 10 insertions(+), 30 deletions(-)

diff --git a/lib/igt_vgem.c b/lib/igt_vgem.c
index 468383c7..93c8398e 100644
--- a/lib/igt_vgem.c
+++ b/lib/igt_vgem.c
@@ -94,41 +94,23 @@ void *vgem_mmap(int fd, struct vgem_bo *bo, unsigned prot)
 	return ptr;
 }
 
-#define LOCAL_VGEM_FENCE_ATTACH   0x1
-#define LOCAL_VGEM_FENCE_SIGNAL   0x2
-
-#define LOCAL_IOCTL_VGEM_FENCE_ATTACH     DRM_IOWR( DRM_COMMAND_BASE + LOCAL_VGEM_FENCE_ATTACH, struct local_vgem_fence_attach)
-#define LOCAL_IOCTL_VGEM_FENCE_SIGNAL     DRM_IOW( DRM_COMMAND_BASE + LOCAL_VGEM_FENCE_SIGNAL, struct local_vgem_fence_signal)
-
-struct local_vgem_fence_attach {
-	uint32_t handle;
-	uint32_t flags;
-	uint32_t out_fence;
-	uint32_t pad;
-};
-
-struct local_vgem_fence_signal {
-	uint32_t fence;
-	uint32_t flags;
-};
-
 bool vgem_has_fences(int fd)
 {
-	struct local_vgem_fence_signal arg;
+	struct drm_vgem_fence_signal arg;
 	int err;
 
 	err = 0;
 	memset(&arg, 0, sizeof(arg));
-	if (drmIoctl(fd, LOCAL_IOCTL_VGEM_FENCE_SIGNAL, &arg))
+	if (drmIoctl(fd, DRM_IOCTL_VGEM_FENCE_SIGNAL, &arg))
 		err = -errno;
 	errno = 0;
 	return err == -ENOENT;
 }
 
-static int __vgem_fence_attach(int fd, struct local_vgem_fence_attach *arg)
+static int __vgem_fence_attach(int fd, struct drm_vgem_fence_attach *arg)
 {
 	int err = 0;
-	if (igt_ioctl(fd, LOCAL_IOCTL_VGEM_FENCE_ATTACH, arg))
+	if (igt_ioctl(fd, DRM_IOCTL_VGEM_FENCE_ATTACH, arg))
 		err = -errno;
 	errno = 0;
 	return err;
@@ -136,7 +118,7 @@ static int __vgem_fence_attach(int fd, struct local_vgem_fence_attach *arg)
 
 bool vgem_fence_has_flag(int fd, unsigned flags)
 {
-	struct local_vgem_fence_attach arg;
+	struct drm_vgem_fence_attach arg;
 	struct vgem_bo bo;
 	bool ret = false;
 
@@ -160,7 +142,7 @@ bool vgem_fence_has_flag(int fd, unsigned flags)
 
 uint32_t vgem_fence_attach(int fd, struct vgem_bo *bo, unsigned flags)
 {
-	struct local_vgem_fence_attach arg;
+	struct drm_vgem_fence_attach arg;
 
 	memset(&arg, 0, sizeof(arg));
 	arg.handle = bo->handle;
@@ -169,10 +151,10 @@ uint32_t vgem_fence_attach(int fd, struct vgem_bo *bo, unsigned flags)
 	return arg.out_fence;
 }
 
-static int ioctl_vgem_fence_signal(int fd, struct local_vgem_fence_signal *arg)
+static int ioctl_vgem_fence_signal(int fd, struct drm_vgem_fence_signal *arg)
 {
 	int err = 0;
-	if (igt_ioctl(fd, LOCAL_IOCTL_VGEM_FENCE_SIGNAL, arg))
+	if (igt_ioctl(fd, DRM_IOCTL_VGEM_FENCE_SIGNAL, arg))
 		err = -errno;
 	errno = 0;
 	return err;
@@ -180,7 +162,7 @@ static int ioctl_vgem_fence_signal(int fd, struct local_vgem_fence_signal *arg)
 
 int __vgem_fence_signal(int fd, uint32_t fence)
 {
-	struct local_vgem_fence_signal arg;
+	struct drm_vgem_fence_signal arg;
 
 	memset(&arg, 0, sizeof(arg));
 	arg.fence = fence;
diff --git a/lib/igt_vgem.h b/lib/igt_vgem.h
index 92045f0e..a9220df7 100644
--- a/lib/igt_vgem.h
+++ b/lib/igt_vgem.h
@@ -24,8 +24,7 @@
 #ifndef IGT_VGEM_H
 #define IGT_VGEM_H
 
-#include <stdint.h>
-#include <stdbool.h>
+#include "vgem_drm.h"
 
 struct vgem_bo {
 	uint32_t handle;
@@ -43,7 +42,6 @@ void *vgem_mmap(int fd, struct vgem_bo *bo, unsigned prot);
 bool vgem_has_fences(int fd);
 bool vgem_fence_has_flag(int fd, unsigned flags);
 uint32_t vgem_fence_attach(int fd, struct vgem_bo *bo, unsigned flags);
-#define VGEM_FENCE_WRITE 0x1
 #define WIP_VGEM_FENCE_NOTIMEOUT 0x2
 int __vgem_fence_signal(int fd, uint32_t fence);
 void vgem_fence_signal(int fd, uint32_t fence);
-- 
2.39.2

[igt-dev] [PATCH i-g-t 2/2] tests/vgem_basic: Add negative tests

[Maíra Canal]

Currently, the vgem_basic tests don't cover any negative tests. So, add
tests covering possible error paths of the driver by inputting invalid
arguments and checking if the driver is returning the correct values.

Signed-off-by: Maíra Canal <mcanal@igalia.com>
---
 tests/vgem_basic.c | 63 ++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 63 insertions(+)

diff --git a/tests/vgem_basic.c b/tests/vgem_basic.c
index 2a2900f8..4f371b3d 100644
--- a/tests/vgem_basic.c
+++ b/tests/vgem_basic.c
@@ -152,6 +152,27 @@ static void test_dmabuf_export(int fd)
 	close(other);
 }
 
+static void test_busy_fence(int fd)
+{
+	struct drm_vgem_fence_attach arg = {};
+	struct vgem_bo bo;
+
+	bo.width = 1024;
+	bo.height = 1;
+	bo.bpp = 32;
+	vgem_create(fd, &bo);
+
+	/* Attach a fence for reading */
+	vgem_fence_attach(fd, &bo, 0);
+
+	/* Attach a fence for writing, so it should be an exclusive fence */
+	arg.handle = bo.handle;
+	arg.flags = VGEM_FENCE_WRITE;
+
+	/* As the fence is not exclusive, return -EBUSY, indicating a conflicting fence */
+	do_ioctl_err(fd, DRM_IOCTL_VGEM_FENCE_ATTACH, &arg, EBUSY);
+}
+
 static void test_dmabuf_mmap(int fd)
 {
 	struct vgem_bo bo;
@@ -434,6 +455,48 @@ igt_main
 	igt_subtest_f("mmap")
 		test_mmap(fd);
 
+	igt_describe("Make sure a fence cannot be attached and signaled with invalid flags.");
+	igt_subtest("bad-flag") {
+		struct drm_vgem_fence_attach attach = {
+			.flags = 0xff,
+		};
+
+		struct drm_vgem_fence_signal signal = {
+			.flags = 0xff,
+		};
+
+		do_ioctl_err(fd, DRM_IOCTL_VGEM_FENCE_ATTACH, &attach, EINVAL);
+		do_ioctl_err(fd, DRM_IOCTL_VGEM_FENCE_SIGNAL, &signal, EINVAL);
+	}
+
+	igt_describe("Make sure the pad is zero.");
+	igt_subtest("bad-pad") {
+		struct drm_vgem_fence_attach arg = {
+			.pad = 0x01,
+		};
+		do_ioctl_err(fd, DRM_IOCTL_VGEM_FENCE_ATTACH, &arg, EINVAL);
+	}
+
+	igt_describe("Make sure a fence cannot be attached to a invalid handle.");
+	igt_subtest("bad-handle") {
+		struct drm_vgem_fence_attach arg = {
+			.handle = 0xff,
+		};
+		do_ioctl_err(fd, DRM_IOCTL_VGEM_FENCE_ATTACH, &arg, ENOENT);
+	}
+
+	igt_describe("Make sure a non-existent fence cannot be signaled.");
+	igt_subtest("bad-fence") {
+		struct drm_vgem_fence_signal arg = {
+			.fence = 0xff,
+		};
+		do_ioctl_err(fd, DRM_IOCTL_VGEM_FENCE_SIGNAL, &arg, ENOENT);
+	}
+
+	igt_describe("Make sure a conflicting fence cannot be attached.");
+	igt_subtest("busy-fence")
+		test_busy_fence(fd);
+
 	igt_subtest_group {
 		igt_fixture {
 			igt_require(has_prime_export(fd));
-- 
2.39.2

[igt-dev] ✗ Fi.CI.BUILD: failure for Add negative tests to VGEM

[Patchwork]

== Series Details ==

Series: Add negative tests to VGEM
URL   : https://patchwork.freedesktop.org/series/114912/
State : failure

== Summary ==

IGT patchset build failed on latest successful build
b35bfa32fe672d67ced8555557e3e707ace211ad runner/job_list: return error on crashes while running --list-subtests

ninja: Entering directory `/opt/igt/build'
[1/1010] Generating version.h with a custom command.
[2/1008] Linking static target lib/libigt-i915_gem_engine_topology_c.a.
[3/1008] Linking static target lib/libigt-i915_intel_memory_region_c.a.
[4/1008] Linking static target lib/libigt-i915_intel_decode_c.a.
[5/1008] Linking static target lib/libigt-igt_collection_c.a.
[6/1008] Linking static target lib/libigt-i915_i915_blt_c.a.
[7/1008] Linking static target lib/libigt-igt_debugfs_c.a.
[8/1008] Linking static target lib/libigt-igt_device_c.a.
[9/1008] Linking static target lib/libigt-igt_device_scan_c.a.
[10/1008] Linking static target lib/libigt-igt_aux_c.a.
[11/1008] Linking static target lib/libigt-igt_gt_c.a.
[12/1008] Linking static target lib/libigt-igt_halffloat_c.a.
[13/1008] Linking static target lib/libigt-igt_os_c.a.
[14/1008] Linking static target lib/libigt-igt_perf_c.a.
[15/1008] Linking static target lib/libigt-igt_pipe_crc_c.a.
[16/1008] Linking static target lib/libigt-igt_power_c.a.
[17/1008] Linking static target lib/libigt-igt_stats_c.a.
[18/1008] Linking static target lib/libigt-igt_syncobj_c.a.
[19/1008] Linking static target lib/libigt-igt_sysfs_c.a.
[20/1008] Linking static target lib/libigt-igt_x86_c.a.
[21/1008] Compiling C object 'lib/76b5a35@@igt-igt_vgem_c@sta/igt_vgem.c.o'.
FAILED: lib/76b5a35@@igt-igt_vgem_c@sta/igt_vgem.c.o 
cc -Ilib/76b5a35@@igt-igt_vgem_c@sta -Ilib -I../../../usr/src/igt-gpu-tools/lib -I../../../usr/src/igt-gpu-tools/include -I../../../usr/src/igt-gpu-tools/include/drm-uapi -I../../../usr/src/igt-gpu-tools/include/linux-uapi -I../../../usr/src/igt-gpu-tools/lib/stubs/syscalls -I. -I../../../usr/src/igt-gpu-tools/ -I/usr/include/cairo -I/usr/include/glib-2.0 -I/usr/lib/x86_64-linux-gnu/glib-2.0/include -I/usr/include/pixman-1 -I/usr/include/uuid -I/usr/include/freetype2 -I/usr/include/libpng16 -I/usr/include/libdrm -I/usr/include/x86_64-linux-gnu -I/usr/include/valgrind -I/usr/include -fdiagnostics-color=always -pipe -D_FILE_OFFSET_BITS=64 -Wall -Winvalid-pch -Wextra -std=gnu11 -O2 -g -D_GNU_SOURCE -include config.h -D_FORTIFY_SOURCE=2 -Wbad-function-cast -Wdeclaration-after-statement -Wformat=2 -Wimplicit-fallthrough=0 -Wlogical-op -Wmissing-declarations -Wmissing-format-attribute -Wmissing-noreturn -Wmissing-prototypes -Wnested-externs -Wold-style-definition -Wpointer-arith -Wredundant-decls -Wshadow -Wstrict-prototypes -Wuninitialized -Wunused -Wno-clobbered -Wno-maybe-uninitialized -Wno-missing-field-initializers -Wno-pointer-arith -Wno-address-of-packed-member -Wno-sign-compare -Wno-type-limits -Wno-unused-parameter -Wno-unused-result -Werror=address -Werror=array-bounds -Werror=implicit -Werror=init-self -Werror=int-to-pointer-cast -Werror=main -Werror=missing-braces -Werror=nonnull -Werror=pointer-to-int-cast -Werror=return-type -Werror=sequence-point -Werror=trigraphs -Werror=write-strings -fno-builtin-malloc -fno-builtin-calloc -fPIC -pthread '-DIGT_DATADIR="/opt/igt/share/igt-gpu-tools"' '-DIGT_SRCDIR="/usr/src/igt-gpu-tools/tests"' '-DIGT_LOG_DOMAIN="igt_vgem"' -MD -MQ 'lib/76b5a35@@igt-igt_vgem_c@sta/igt_vgem.c.o' -MF 'lib/76b5a35@@igt-igt_vgem_c@sta/igt_vgem.c.o.d' -o 'lib/76b5a35@@igt-igt_vgem_c@sta/igt_vgem.c.o' -c ../../../usr/src/igt-gpu-tools/lib/igt_vgem.c
In file included from ../../../usr/src/igt-gpu-tools/lib/igt_vgem.c:29:
../../../usr/src/igt-gpu-tools/lib/igt_vgem.h:42:1: error: unknown type name ‘bool’
   42 | bool vgem_has_fences(int fd);
      | ^~~~
../../../usr/src/igt-gpu-tools/lib/igt_vgem.h:43:1: error: unknown type name ‘bool’
   43 | bool vgem_fence_has_flag(int fd, unsigned flags);
      | ^~~~
../../../usr/src/igt-gpu-tools/lib/igt_vgem.c:97:6: error: conflicting types for ‘vgem_has_fences’
   97 | bool vgem_has_fences(int fd)
      |      ^~~~~~~~~~~~~~~
In file included from ../../../usr/src/igt-gpu-tools/lib/igt_vgem.c:29:
../../../usr/src/igt-gpu-tools/lib/igt_vgem.h:42:6: note: previous declaration of ‘vgem_has_fences’ was here
   42 | bool vgem_has_fences(int fd);
      |      ^~~~~~~~~~~~~~~
../../../usr/src/igt-gpu-tools/lib/igt_vgem.c:119:6: error: conflicting types for ‘vgem_fence_has_flag’
  119 | bool vgem_fence_has_flag(int fd, unsigned flags)
      |      ^~~~~~~~~~~~~~~~~~~
In file included from ../../../usr/src/igt-gpu-tools/lib/igt_vgem.c:29:
../../../usr/src/igt-gpu-tools/lib/igt_vgem.h:43:6: note: previous declaration of ‘vgem_fence_has_flag’ was here
   43 | bool vgem_fence_has_flag(int fd, unsigned flags);
      |      ^~~~~~~~~~~~~~~~~~~
ninja: build stopped: subcommand failed.