Re: [PATCH v2 i-g-t] igt/perf: add tests to verify create/destroy userspace configs

Lionel Landwerlin <lionel.g.landwerlin@xxxxxxxxx> · Mon, 17 Jul 2017 10:16:21 +0100

On 14/07/17 19:21, Matthew Auld wrote:
On 14 July 2017 at 18:57, Lionel Landwerlin
<lionel.g.landwerlin@xxxxxxxxx> wrote:
On 14/07/17 18:09, Matthew Auld wrote:
On 13 July 2017 at 12:16, Lionel Landwerlin
<lionel.g.landwerlin@xxxxxxxxx> wrote:
v2: Add tests regarding removing configs (Matthew)
      Add tests regarding adding/removing configs without permissions
      (Matthew)

Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@xxxxxxxxx>
---
   tests/perf.c | 201
+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
   1 file changed, 201 insertions(+)
I still think it would be nice to have at least have some flex tests,
especially if we consider the issue with not reserving enough dwords,
even if it's a pita...

Well the current test were testing exactly that, and we didn't see any
errors.
Because in most cases, the back of the batchbuffer will just get rewritten
by the next batch.
Assuming we don't reserve enough dwords then intel_ring_advance()
should catch this, it checks the advanced cs against the number of
reserved dwords from intel_ring_begin(), so we should be hitting the
GEM_BUG_ON(), unless I'm glossing over something. Do you have
DEBUG_GEM enabled?

Ah, that must be why...



Apart from testing the if (reg >= xxx && reg =< yyy), I don't know that we
can test anything interesting here :(
I was thinking flex configs with n_regs < max_flex, and also n_regs > max_flex.

Ok, will add n_regs > max_flex.
n_regs < max_flex should be covered as we currently don't have any flex reg.

diff --git a/tests/perf.c b/tests/perf.c
index db28ba1f..47caf1c6 100644
--- a/tests/perf.c
+++ b/tests/perf.c
@@ -146,6 +146,36 @@ enum drm_i915_perf_record_type {
   };
   #endif /* !DRM_I915_PERF_OPEN */

+#ifndef DRM_IOCTL_I915_PERF_ADD_CONFIG
+
+#define DRM_I915_PERF_ADD_CONFIG       0x37
+#define DRM_I915_PERF_REMOVE_CONFIG    0x38
+
+#define DRM_IOCTL_I915_PERF_ADD_CONFIG DRM_IOWR(DRM_COMMAND_BASE +
DRM_I915_PERF_ADD_CONFIG, struct drm_i915_perf_oa_config)
+#define DRM_IOCTL_I915_PERF_REMOVE_CONFIG      DRM_IOWR(DRM_COMMAND_BASE
+ DRM_I915_PERF_REMOVE_CONFIG, __u64)
+
+/**
+ * Structure to upload perf dynamic configuration into the kernel.
+ */
+struct drm_i915_perf_oa_config {
+       /** String formatted like "%08x-%04x-%04x-%04x-%012x" */
+       __u64 uuid;
+
+       __u32 n_mux_regs;
+       __u32 pad0;
+       __u64 mux_regs;
+
+       __u32 n_boolean_regs;
+       __u32 pad1;
+       __u64 boolean_regs;
+
+       __u32 n_flex_regs;
+       __u32 pad2;
+       __u64 flex_regs;
+};
+
+#endif /* !DRM_IOCTL_I915_PERF_ADD_CONFIG */
+
   struct accumulator {
   #define MAX_RAW_OA_COUNTERS 62
          enum drm_i915_oa_format format;
@@ -4001,6 +4031,168 @@ test_rc6_disable(void)
          igt_assert_neq(n_events_end - n_events_start, 0);
   }

+static void
+test_invalid_userspace_config_create(void)
+{
+       struct drm_i915_perf_oa_config config;
+       const char *uuid = "01234567-0123-0123-0123-0123456789ab";
+       const char *invalid_uuid = "blablabla-wrong";
+       uint32_t mux_regs[] = { 0x9888 /* NOA_WRITE */, 0x0 };
+       uint32_t invalid_mux_regs[] = { 0x12345678 /* invalid register
*/, 0x0 };
+
+       memset(&config, 0, sizeof(config));
+
+       /* invalid uuid */
+       config.uuid = to_user_pointer(invalid_uuid);
+       config.n_mux_regs = 1;
+       config.mux_regs = to_user_pointer(mux_regs);
+       config.n_boolean_regs = 0;
+       config.n_flex_regs = 0;
+
+       do_ioctl_err(drm_fd, DRM_IOCTL_I915_PERF_ADD_CONFIG, &config,
EINVAL);
+
+       /* invalid mux_regs */
+       config.uuid = to_user_pointer(uuid);
+       config.n_mux_regs = 1;
+       config.mux_regs = to_user_pointer(invalid_mux_regs);
+       config.n_boolean_regs = 0;
+       config.n_flex_regs = 0;
+
+       do_ioctl_err(drm_fd, DRM_IOCTL_I915_PERF_ADD_CONFIG, &config,
EINVAL);
+
+       /* empty config */
+       config.uuid = to_user_pointer(uuid);
+       config.n_mux_regs = 0;
+       config.mux_regs = to_user_pointer(mux_regs);
+       config.n_boolean_regs = 0;
+       config.n_flex_regs = 0;
+
+       do_ioctl_err(drm_fd, DRM_IOCTL_I915_PERF_ADD_CONFIG, &config,
EINVAL);
+
+       /* empty config with null pointers */
+       config.uuid = to_user_pointer(uuid);
+       config.n_mux_regs = 1;
+       config.mux_regs = to_user_pointer(NULL);
+       config.n_boolean_regs = 2;
+       config.boolean_regs = to_user_pointer(NULL);
+       config.n_flex_regs = 3;
+       config.flex_regs = to_user_pointer(NULL);
+
+       do_ioctl_err(drm_fd, DRM_IOCTL_I915_PERF_ADD_CONFIG, &config,
EINVAL);
+}
+
+static void
+test_invalid_userspace_config_remove(void)
+{
+       struct drm_i915_perf_oa_config config;
+       const char *uuid = "01234567-0123-0123-0123-0123456789ab";
+       uint32_t mux_regs[] = { 0x9888 /* NOA_WRITE */, 0x0 };
+       uint64_t config_id, wrong_config_id = 999999999;
+       char path[512];
+       int ret;
+
+       snprintf(path, sizeof(path),
"/sys/class/drm/card%d/metrics/%s/id", card, uuid);
+
+       /* Destroy previous configuration if present */
+       if (try_read_u64_file(path, &config_id))
+         igt_assert(igt_ioctl(drm_fd, DRM_IOCTL_I915_PERF_REMOVE_CONFIG,
&config_id) == 0);
+
+       memset(&config, 0, sizeof(config));
+
+       config.uuid = to_user_pointer(uuid);
+
+       config.n_mux_regs = 1;
+       config.mux_regs = to_user_pointer(mux_regs);
+       config.n_boolean_regs = 0;
+       config.n_flex_regs = 0;
+
+       ret = igt_ioctl(drm_fd, DRM_IOCTL_I915_PERF_ADD_CONFIG, &config);
+       igt_assert(ret > 0);
+       config_id = ret;
+
+       /* Removing configs without permissions should fail. */
+       igt_fork(child, 1) {
+               igt_drop_root();
+
+               do_ioctl_err(drm_fd, DRM_IOCTL_I915_PERF_REMOVE_CONFIG,
&config_id, EACCES);
+       }
+       igt_waitchildren();
+
+       /* Removing invalid config ID should fail. */
+       do_ioctl_err(drm_fd, DRM_IOCTL_I915_PERF_REMOVE_CONFIG,
&wrong_config_id, EINVAL);
+
+       igt_assert(igt_ioctl(drm_fd, DRM_IOCTL_I915_PERF_REMOVE_CONFIG,
&config_id) == 0);
+}
+
+static void
+test_create_destroy_userspace_config(void)
+{
+       struct drm_i915_perf_oa_config config;
+       const char *uuid = "01234567-0123-0123-0123-0123456789ab";
+       uint32_t mux_regs[] = { 0x9888 /* NOA_WRITE */, 0x0 };
+       int ret;
+       uint64_t config_id;
+       uint64_t properties[] = {
+               DRM_I915_PERF_PROP_OA_METRICS_SET, 0, /* Filled later */
+
+               /* OA unit configuration */
+               DRM_I915_PERF_PROP_SAMPLE_OA, true,
+               DRM_I915_PERF_PROP_OA_FORMAT, test_oa_format,
+               DRM_I915_PERF_PROP_OA_EXPONENT, oa_exp_1_millisec,
+               DRM_I915_PERF_PROP_OA_METRICS_SET
+       };
+       struct drm_i915_perf_open_param param = {
+               .flags = I915_PERF_FLAG_FD_CLOEXEC |
+               I915_PERF_FLAG_FD_NONBLOCK |
+               I915_PERF_FLAG_DISABLED,
+               .num_properties = ARRAY_SIZE(properties) / 2,
+               .properties_ptr = to_user_pointer(properties),
+       };
+       char path[512];
+
+       snprintf(path, sizeof(path),
"/sys/class/drm/card%d/metrics/%s/id", card, uuid);
+
+       /* Destroy previous configuration if present */
+       if (try_read_u64_file(path, &config_id))
+         igt_assert(igt_ioctl(drm_fd, DRM_IOCTL_I915_PERF_REMOVE_CONFIG,
&config_id) == 0);
+
+       config.uuid = to_user_pointer(uuid);
+
+       config.n_mux_regs = 1;
+       config.mux_regs = to_user_pointer(mux_regs);
+       config.n_boolean_regs = 0;
+       config.n_flex_regs = 0;
+
+       /* Creating configs without permissions shouldn't work. */
+       igt_fork(child, 1) {
+               igt_drop_root();
+
+               do_ioctl_err(drm_fd, DRM_IOCTL_I915_PERF_ADD_CONFIG,
&config, EACCES);
+       }
+       igt_waitchildren();
+
+       /* Create a new config */
+       ret = igt_ioctl(drm_fd, DRM_IOCTL_I915_PERF_ADD_CONFIG, &config);
+       igt_assert(ret > 0); /* Config 0 should be used by the kernel */
+       config_id = ret;
+
+       /* Verify that adding the another config with the same uuid
fails. */
+       do_ioctl_err(drm_fd, DRM_IOCTL_I915_PERF_ADD_CONFIG, &config,
EADDRINUSE);
+
+       /* Try to use the new config */
+       properties[1] = config_id;
+       stream_fd = __perf_open(drm_fd, &param);
+
+       /* Verify that destroying the config doesn't work while it's
being
+        * used.
+        */
+       do_ioctl_err(drm_fd, DRM_IOCTL_I915_PERF_REMOVE_CONFIG,
&config_id, EADDRINUSE);
+
+       __perf_close(stream_fd);
+
+       igt_assert(igt_ioctl(drm_fd, DRM_IOCTL_I915_PERF_REMOVE_CONFIG,
&config_id) == 0);
+}
+
   static unsigned
   read_i915_module_ref(void)
   {
@@ -4223,6 +4415,15 @@ igt_main
          igt_subtest("rc6-disable")
                  test_rc6_disable();

+       igt_subtest("invalid-userspace-config-create")
+               test_invalid_userspace_config_create();
+
+       igt_subtest("invalid-userspace-config-remove")
+               test_invalid_userspace_config_remove();
+
+       igt_subtest("create-destroy-userspace-config")
+               test_create_destroy_userspace_config();
+
          igt_fixture {
                  /* leave sysctl options in their default state... */
                  write_u64_file("/proc/sys/dev/i915/oa_max_sample_rate",
100000);
--
2.13.2
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx



_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx







