[Mesa-dev] [PATCH 4/5] i965: perf: add support for userspace configurations

[Lionel Landwerlin]

On 29/08/17 12:15, Chris Wilson wrote:
> Quoting Lionel Landwerlin (2017-08-29 11:58:57)
>> This allows us to deploy new configurations without touching the
>> kernel.
>>
>> Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin at intel.com>
>> ---
>>   src/mesa/drivers/dri/i965/brw_performance_query.c | 97 ++++++++++++++++++++++-
>>   1 file changed, 96 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/mesa/drivers/dri/i965/brw_performance_query.c b/src/mesa/drivers/dri/i965/brw_performance_query.c
>> index 4b585c95b7d..901cbb464e9 100644
>> --- a/src/mesa/drivers/dri/i965/brw_performance_query.c
>> +++ b/src/mesa/drivers/dri/i965/brw_performance_query.c
>> @@ -1807,6 +1807,96 @@ read_sysfs_drm_device_file_uint64(struct brw_context *brw,
>>      return read_file_uint64(buf, value);
>>   }
>>   
>> +static bool
>> +kernel_has_dynamic_config_support(int drm_fd, const char *sysfs_dev_dir)
>> +{
>> +    struct drm_i915_perf_oa_config config;
>> +    const char *uuid = "01234567-0123-0123-0123-0123456789ab";
>> +    uint32_t mux_regs[] = { 0x9888 /* NOA_WRITE */, 0x0 };
>> +    char config_path[256];
>> +    struct stat sb;
>> +    uint64_t config_id;
>> +    int ret;
>> +
>> +    snprintf(config_path, sizeof(config_path),
>> +             "%s/metrics/%s/id", sysfs_dev_dir, uuid);
>> +
>> +    if (stat(config_path, &sb) == 0) {
>> +       if (!read_file_uint64(config_path, &config_id))
>> +          return false;
>> +
>> +       if (ioctl(drm_fd, DRM_IOCTL_I915_PERF_REMOVE_CONFIG, &config_id) == 0)
>> +          return true;
> So you acknowledge the race condition with another process also doing
> the same test, or worse if they genuinely used that uuid for an actual
> config.

Thanks, I should randomize that.

>
>> +    }
>> +
>> +    memset(&config, 0, sizeof(config));
>> +    memcpy(config.uuid, uuid, sizeof(config.uuid));
>> +
>> +    config.n_mux_regs = 1;
>> +    config.mux_regs_ptr = (uintptr_t) mux_regs;
>> +
>> +    /* Create a new config */
>> +    ret = ioctl(drm_fd, DRM_IOCTL_I915_PERF_ADD_CONFIG, &config);
>> +    if (ret < 0)
>> +        return false;
> A trick you may like to try is to create an invalid config that
> generates something other than EINVAL so that you can then distinguish
> with the non-existent ioctl cmd error. Since you reserved id=0 for
> invalid, a good choice would be
>
> static bool has_dyn_config() {
> 	uint64_t config_id = 0;
> 	return sys_ioctl(drm_fd, DRM_IOCTL_I915_PERF_REMOVE_CONFIG, &config_id) == -ENOENT;
> }

Thanks!

> -Chris
>