[igt-dev] [PATCH i-g-t] tests/kms_properties: Validate properties harder
Ville Syrjala
ville.syrjala at linux.intel.com
Wed Mar 7 21:33:07 UTC 2018
From: Ville Syrjälä <ville.syrjala at linux.intel.com>
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 at linux.intel.com>
---
lib/igt_aux.h | 2 +
lib/igt_core.h | 52 +++++++
tests/kms_properties.c | 364 +++++++++++++++++++++++++++++++++++++++++++------
3 files changed, 374 insertions(+), 44 deletions(-)
diff --git a/lib/igt_aux.h b/lib/igt_aux.h
index 43dd15fe3b32..b261060ed565 100644
--- a/lib/igt_aux.h
+++ b/lib/igt_aux.h
@@ -385,4 +385,6 @@ static inline bool igt_list_empty(const struct igt_list *list)
&pos->member != (head); \
pos = tmp, tmp = igt_list_next_entry(pos, member))
+#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 66523a208c31..17c9371cf6f7 100644
--- a/lib/igt_core.h
+++ b/lib/igt_core.h
@@ -575,6 +575,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
@@ -588,6 +614,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 a63b69b4decf..68c0518fca5f 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);
--
2.16.1
More information about the igt-dev
mailing list