tests/kms_properties: Validate properties harder

Make the property validation more thorough:
- validate property flags
- make sure there's an expected number of values/enums
- make sure the possible values make sense
- make sure the current value makes sense
- actually iterate through all planes/crtc/connectors to
  check their properties
- make sure encoders don't expose properties while at it
- check that atomic props aren't exposed to non-atomic clients

Still passes on my ivb. Not tested anything else so far.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>

3 files changed, 374 insertions, 44 deletions

diff --git a/lib/igt_aux.h b/lib/igt_aux.h
index faddd478..9bb03b77 100644
--- a/lib/igt_aux.h
+++ b/lib/igt_aux.h
@@ -390,4 +390,6 @@ static inline bool igt_list_empty(const struct igt_list *list)
 			      __builtin_popcountll(x), \
 			      __builtin_popcount(x))
 
+#define is_power_of_two(x)  (((x) & ((x)-1)) == 0)
+
 #endif /* IGT_AUX_H */
diff --git a/lib/igt_core.h b/lib/igt_core.h
index 57e97f2b..bf8cd66c 100644
--- a/lib/igt_core.h
+++ b/lib/igt_core.h
@@ -576,6 +576,32 @@ static inline void igt_ignore_warn(bool value)
 #define igt_assert_lte(n1, n2) igt_assert_cmpint(n1, <=, >, n2)
 
 /**
+ * igt_assert_lte_u64:
+ * @n1: first integer
+ * @n2: second integer
+ *
+ * Fails (sub-)test if the second integer is strictly smaller than the first.
+ * Beware that for now this only works on integers.
+ *
+ * Like igt_assert(), but displays the values being compared on failure instead
+ * of simply printing the stringified expression.
+ */
+#define igt_assert_lte_u64(n1, n2) igt_assert_cmpu64(n1, <=, >, n2)
+
+/**
+ * igt_assert_lte_s64:
+ * @n1: first integer
+ * @n2: second integer
+ *
+ * Fails (sub-)test if the second integer is strictly smaller than the first.
+ * Beware that for now this only works on integers.
+ *
+ * Like igt_assert(), but displays the values being compared on failure instead
+ * of simply printing the stringified expression.
+ */
+#define igt_assert_lte_s64(n1, n2) igt_assert_cmps64(n1, <=, >, n2)
+
+/**
  * igt_assert_lt:
  * @n1: first integer
  * @n2: second integer
@@ -589,6 +615,32 @@ static inline void igt_ignore_warn(bool value)
 #define igt_assert_lt(n1, n2) igt_assert_cmpint(n1, <, >=, n2)
 
 /**
+ * igt_assert_lt_u64:
+ * @n1: first integer
+ * @n2: second integer
+ *
+ * Fails (sub-)test if the second integer is smaller than or equal to the first.
+ * Beware that for now this only works on integers.
+ *
+ * Like igt_assert(), but displays the values being compared on failure instead
+ * of simply printing the stringified expression.
+ */
+#define igt_assert_lt_u64(n1, n2) igt_assert_cmpu64(n1, <, >=, n2)
+
+/**
+ * igt_assert_lt_s64:
+ * @n1: first integer
+ * @n2: second integer
+ *
+ * Fails (sub-)test if the second integer is smaller than or equal to the first.
+ * Beware that for now this only works on integers.
+ *
+ * Like igt_assert(), but displays the values being compared on failure instead
+ * of simply printing the stringified expression.
+ */
+#define igt_assert_lt_s64(n1, n2) igt_assert_cmps64(n1, <, >=, n2)
+
+/**
  * igt_assert_fd:
  * @fd: file descriptor
  *
diff --git a/tests/kms_properties.c b/tests/kms_properties.c
index a63b69b4..68c0518f 100644
--- a/tests/kms_properties.c
+++ b/tests/kms_properties.c
@@ -325,63 +325,327 @@ static void test_object_invalid_properties(igt_display_t *display,
 		test_invalid_properties(display->drm_fd, id, type, output->id, DRM_MODE_OBJECT_CONNECTOR, atomic);
 }
 
-static void get_prop_sanity(igt_display_t *display)
+static void validate_range_prop(const struct drm_mode_get_property *prop,
+				uint64_t value)
 {
-	int i, fd;
-	uint64_t *values;
-	struct drm_mode_property_enum *enums;
+	const uint64_t *values = from_user_pointer(prop->values_ptr);
+	bool is_unsigned = prop->flags & DRM_MODE_PROP_RANGE;
+	bool immutable = prop->flags & DRM_MODE_PROP_IMMUTABLE;
+
+	igt_assert_eq(prop->count_values, 2);
+	igt_assert_eq(prop->count_enum_blobs, 0);
+	igt_assert(values[0] != values[1] || immutable);
+
+	if (is_unsigned) {
+		igt_assert_lte_u64(values[0], values[1]);
+		igt_assert_lte_u64(values[0], value);
+		igt_assert_lte_u64(value, values[1]);
+	} else {
+		igt_assert_lte_s64(values[0], values[1]);
+		igt_assert_lte_s64(values[0], value);
+		igt_assert_lte_s64(value, values[1]);
+	}
+
+}
+
+static void validate_enums(const struct drm_mode_get_property *prop)
+{
+	const uint64_t *values = from_user_pointer(prop->values_ptr);
+	const struct drm_mode_property_enum *enums =
+		from_user_pointer(prop->enum_blob_ptr);
+
+	for (int i = 0; i < prop->count_enum_blobs; i++) {
+		int name_len = strnlen(enums[i].name,
+				       sizeof(enums[i].name));
 
-	fd = display->drm_fd;
+		igt_assert_lte(1, name_len);
+		igt_assert_lte(name_len, sizeof(enums[i].name) - 1);
+
+		/* no idea why we have this duplicated */
+		igt_assert_eq_u64(values[i], enums[i].value);
+	}
+}
+
+static void validate_enum_prop(const struct drm_mode_get_property *prop,
+			       uint64_t value)
+{
+	const uint64_t *values = from_user_pointer(prop->values_ptr);
+	bool immutable = prop->flags & DRM_MODE_PROP_IMMUTABLE;
+	int i;
+
+	igt_assert_lte(1, prop->count_values);
+	igt_assert_eq(prop->count_enum_blobs, prop->count_values);
+	igt_assert(prop->count_values != 1 || immutable);
+
+	for (i = 0; i < prop->count_values; i++) {
+		if (value == values[i])
+			break;
+	}
+	igt_assert(i != prop->count_values);
+
+	validate_enums(prop);
+}
+
+static void validate_bitmask_prop(const struct drm_mode_get_property *prop,
+				  uint64_t value)
+{
+	const uint64_t *values = from_user_pointer(prop->values_ptr);
+	bool immutable = prop->flags & DRM_MODE_PROP_IMMUTABLE;
+	uint64_t mask = 0;
+
+	igt_assert_lte(1, prop->count_values);
+	igt_assert_eq(prop->count_enum_blobs, prop->count_values);
+	igt_assert(prop->count_values != 1 || immutable);
+
+	for (int i = 0; i < prop->count_values; i++) {
+		igt_assert_lte_u64(values[i], 63);
+		mask |= 1ULL << values[i];
+	}
+
+	igt_assert_eq_u64(value & ~mask, 0);
+	igt_assert_neq_u64(value & mask, 0);
+
+	validate_enums(prop);
+}
+
+static void validate_blob_prop(int fd,
+			       const struct drm_mode_get_property *prop,
+			       uint64_t value)
+{
+	struct drm_mode_get_blob blob;
 
 	/*
-	 * There's no way to enumerate all properties, we just have to
-	 * brute-force the first few kms ids. 1000 should be enough.
+	 * Despite what libdrm makes you believe, we never supply
+	 * additional information for BLOB properties, only for enums
+	 * and bitmasks
 	 */
-	for (i = 0; i < 1000; i++) {
-		struct drm_mode_get_property prop;
+	igt_assert_eq(prop->count_values, 0);
+	igt_assert_eq(prop->count_enum_blobs, 0);
 
-		memset(&prop, 0, sizeof(prop));
-		prop.prop_id = i;
+	igt_assert_lte_u64(value, 0xffffffff);
 
-		if (drmIoctl(fd, DRM_IOCTL_MODE_GETPROPERTY, &prop))
-			continue;
+	/*
+	 * Immutable blob properties can have value==0.
+	 * Happens for example with the "EDID" property
+	 * when there is nothing hooked up to the connector.
+	 */
 
-		if (prop.count_values) {
-			values = calloc(prop.count_values, sizeof(uint64_t));
-			igt_assert(values);
-			memset(values, 0x5c, sizeof(uint64_t)*prop.count_values);
-			prop.values_ptr = to_user_pointer(values);
-		}
+	if (!value)
+		return;
 
-		/* despite what libdrm makes you believe, we never supply
-		 * additional information for BLOB properties, only for enums
-		 * and bitmasks */
-		igt_assert_eq(!!prop.count_enum_blobs,
-			      !!(prop.flags & (DRM_MODE_PROP_ENUM | DRM_MODE_PROP_BITMASK)));
-		if (prop.flags & (DRM_MODE_PROP_ENUM | DRM_MODE_PROP_BITMASK))
-			igt_assert(prop.count_enum_blobs == prop.count_values);
-
-		if (prop.count_enum_blobs) {
-			enums = calloc(prop.count_enum_blobs, sizeof(*enums));
-			memset(enums, 0x5c, sizeof(*enums)*prop.count_enum_blobs);
-			igt_assert(enums);
-			prop.enum_blob_ptr = to_user_pointer(enums);
-		}
+	memset(&blob, 0, sizeof(blob));
+	blob.blob_id = value;
 
-		do_ioctl(fd, DRM_IOCTL_MODE_GETPROPERTY, &prop);
+	do_ioctl(fd, DRM_IOCTL_MODE_GETPROPBLOB, &blob);
+}
 
-		for (int j = 0; j < prop.count_values; j++) {
-			igt_assert(values[j] != 0x5c5c5c5c5c5c5c5cULL);
+static void validate_object_prop(int fd,
+				 const struct drm_mode_get_property *prop,
+				 uint64_t value)
+{
+	const uint64_t *values = from_user_pointer(prop->values_ptr);
+	bool immutable = prop->flags & DRM_MODE_PROP_IMMUTABLE;
+	struct drm_mode_crtc crtc;
+	struct drm_mode_fb_cmd fb;
 
-			if (!(prop.flags & (DRM_MODE_PROP_ENUM | DRM_MODE_PROP_BITMASK)))
-				continue;
-			igt_assert(enums[j].value != 0x5c5c5c5c5c5c5c5cULL);
-			igt_assert(enums[j].value == values[j]);
-			igt_assert(enums[j].name[0] != '\0');
-		}
+	igt_assert_eq(prop->count_values, 1);
+	igt_assert_eq(prop->count_enum_blobs, 0);
+
+	igt_assert_lte_u64(value, 0xffffffff);
+	igt_assert(!immutable || value != 0);
+
+	switch (values[0]) {
+	case DRM_MODE_OBJECT_CRTC:
+		if (!value)
+			break;
+		memset(&crtc, 0, sizeof(crtc));
+		crtc.crtc_id = value;
+		do_ioctl(fd, DRM_IOCTL_MODE_GETCRTC, &crtc);
+		break;
+	case DRM_MODE_OBJECT_FB:
+		if (!value)
+			break;
+		memset(&fb, 0, sizeof(fb));
+		fb.fb_id = value;
+		do_ioctl(fd, DRM_IOCTL_MODE_GETFB, &fb);
+		break;
+	default:
+		/* These are the only types we have so far */
+		igt_assert(0);
 	}
 }
 
+static void validate_property(int fd,
+			      const struct drm_mode_get_property *prop,
+			      uint64_t value, bool atomic)
+{
+	uint32_t flags = prop->flags;
+	uint32_t legacy_type = flags & DRM_MODE_PROP_LEGACY_TYPE;
+	uint32_t ext_type = flags & DRM_MODE_PROP_EXTENDED_TYPE;
+
+	igt_assert_eq((flags & ~(DRM_MODE_PROP_LEGACY_TYPE |
+				 DRM_MODE_PROP_EXTENDED_TYPE |
+				 DRM_MODE_PROP_IMMUTABLE |
+				 DRM_MODE_PROP_ATOMIC)), 0);
+
+	igt_assert(atomic ||
+		   (flags & DRM_MODE_PROP_ATOMIC) == 0);
+
+	igt_assert_neq(!legacy_type, !ext_type);
+
+	igt_assert(legacy_type == 0 ||
+		   is_power_of_two(legacy_type));
+
+	switch (legacy_type) {
+	case DRM_MODE_PROP_RANGE:
+		validate_range_prop(prop, value);
+		break;
+	case DRM_MODE_PROP_ENUM:
+		validate_enum_prop(prop, value);
+		break;
+	case DRM_MODE_PROP_BITMASK:
+		validate_bitmask_prop(prop, value);
+		break;
+	case DRM_MODE_PROP_BLOB:
+		validate_blob_prop(fd, prop, value);
+		break;
+	default:
+		igt_assert_eq(legacy_type, 0);
+	}
+
+	switch (ext_type) {
+	case DRM_MODE_PROP_OBJECT:
+		validate_object_prop(fd, prop, value);
+		break;
+	case DRM_MODE_PROP_SIGNED_RANGE:
+		validate_range_prop(prop, value);
+		break;
+	default:
+		igt_assert_eq(ext_type, 0);
+	}
+}
+
+static void validate_prop(int fd, uint32_t prop_id, uint64_t value, bool atomic)
+{
+	struct drm_mode_get_property prop;
+	struct drm_mode_property_enum *enums = NULL;
+	uint64_t *values = NULL;
+
+	memset(&prop, 0, sizeof(prop));
+	prop.prop_id = prop_id;
+
+	do_ioctl(fd, DRM_IOCTL_MODE_GETPROPERTY, &prop);
+
+	if (prop.count_values) {
+		values = calloc(prop.count_values, sizeof(values[0]));
+		igt_assert(values);
+		memset(values, 0x5c, sizeof(values[0])*prop.count_values);
+		prop.values_ptr = to_user_pointer(values);
+	}
+
+	if (prop.count_enum_blobs) {
+		enums = calloc(prop.count_enum_blobs, sizeof(enums[0]));
+		memset(enums, 0x5c, sizeof(enums[0])*prop.count_enum_blobs);
+		igt_assert(enums);
+		prop.enum_blob_ptr = to_user_pointer(enums);
+	}
+
+	do_ioctl(fd, DRM_IOCTL_MODE_GETPROPERTY, &prop);
+
+	for (int i = 0; i < prop.count_values; i++)
+		igt_assert_neq_u64(values[i], 0x5c5c5c5c5c5c5c5cULL);
+
+	for (int i = 0; i < prop.count_enum_blobs; i++)
+		igt_assert_neq_u64(enums[i].value, 0x5c5c5c5c5c5c5c5cULL);
+
+	validate_property(fd, &prop, value, atomic);
+
+	free(values);
+	free(enums);
+}
+
+static void validate_props(int fd, uint32_t obj_type, uint32_t obj_id, bool atomic)
+{
+	struct drm_mode_obj_get_properties properties;
+	uint32_t *props = NULL;
+	uint64_t *values = NULL;
+	uint32_t count;
+
+	memset(&properties, 0, sizeof(properties));
+	properties.obj_type = obj_type;
+	properties.obj_id = obj_id;
+
+	do_ioctl(fd, DRM_IOCTL_MODE_OBJ_GETPROPERTIES, &properties);
+
+	count = properties.count_props;
+
+	if (count) {
+		props = calloc(count, sizeof(props[0]));
+		memset(props, 0x5c, sizeof(props[0])*count);
+		igt_assert(props);
+		properties.props_ptr = to_user_pointer(props);
+
+		values = calloc(count, sizeof(values[0]));
+		memset(values, 0x5c, sizeof(values[0])*count);
+		igt_assert(values);
+		properties.prop_values_ptr = to_user_pointer(values);
+	}
+
+	do_ioctl(fd, DRM_IOCTL_MODE_OBJ_GETPROPERTIES, &properties);
+
+	igt_assert(properties.count_props == count);
+
+	for (int i = 0; i < count; i++)
+		validate_prop(fd, props[i], values[i], atomic);
+
+	free(values);
+	free(props);
+}
+
+static void expect_no_props(int fd, uint32_t obj_type, uint32_t obj_id)
+{
+	struct drm_mode_obj_get_properties properties;
+
+	memset(&properties, 0, sizeof(properties));
+	properties.obj_type = obj_type;
+	properties.obj_id = obj_id;
+
+	igt_assert_neq(drmIoctl(fd, DRM_IOCTL_MODE_OBJ_GETPROPERTIES, &properties), 0);
+}
+
+static void get_prop_sanity(igt_display_t *display, bool atomic)
+{
+	int fd = display->drm_fd;
+	drmModePlaneResPtr plane_res;
+	drmModeResPtr res;
+
+	res = drmModeGetResources(fd);
+	plane_res = drmModeGetPlaneResources(fd);
+
+	for (int i = 0; i < plane_res->count_planes; i++) {
+		validate_props(fd, DRM_MODE_OBJECT_PLANE,
+			       plane_res->planes[i], atomic);
+	}
+
+	for (int i = 0; i < res->count_crtcs; i++) {
+		validate_props(fd, DRM_MODE_OBJECT_CRTC,
+			       res->crtcs[i], atomic);
+	}
+
+	for (int i = 0; i < res->count_connectors; i++) {
+		validate_props(fd, DRM_MODE_OBJECT_CONNECTOR,
+			       res->connectors[i], atomic);
+	}
+
+	for (int i = 0; i < res->count_encoders; i++) {
+		expect_no_props(fd, DRM_MODE_OBJECT_ENCODER,
+				res->encoders[i]);
+	}
+
+	drmModeFreePlaneResources(plane_res);
+	drmModeFreeResources(res);
+}
+
 static void invalid_properties(igt_display_t *display, bool atomic)
 {
 	igt_output_t *output;
@@ -441,8 +705,20 @@ igt_main
 	igt_subtest("invalid-properties-atomic")
 		invalid_properties(&display, true);
 
-	igt_subtest("get_properties-sanity")
-		get_prop_sanity(&display);
+	igt_subtest("get_properties-sanity-atomic") {
+		igt_skip_on(!display.is_atomic);
+		get_prop_sanity(&display, true);
+	}
+
+	igt_subtest("get_properties-sanity-non-atomic") {
+		if (display.is_atomic)
+			igt_assert_eq(drmSetClientCap(display.drm_fd, DRM_CLIENT_CAP_ATOMIC, 0), 0);
+
+		get_prop_sanity(&display, false);
+
+		if (display.is_atomic)
+			igt_assert_eq(drmSetClientCap(display.drm_fd, DRM_CLIENT_CAP_ATOMIC, 1), 0);
+	}
 
 	igt_fixture {
 		igt_display_fini(&display);