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?
>
> 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.
>
>
>
>>
>>> 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, ¶m);
>>> +
>>> + /* 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
