diff mbox

drm/i915: add support for perf configuration queries

Message ID 20180605161859.26382-1-lionel.g.landwerlin@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Lionel Landwerlin June 5, 2018, 4:18 p.m. UTC
Listing configurations at the moment is supported only through sysfs.
This might cause issues for applications wanting to list
configurations from a container where sysfs isn't available.

This change adds a way to query the number of configurations and their
content through the i915 query uAPI.

Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h   |   6 +
 drivers/gpu/drm/i915/i915_perf.c  |   4 +
 drivers/gpu/drm/i915/i915_query.c | 256 ++++++++++++++++++++++++++++++
 include/uapi/drm/i915_drm.h       |  47 +++++-
 4 files changed, 312 insertions(+), 1 deletion(-)

Comments

kernel test robot June 6, 2018, 6:39 p.m. UTC | #1
Hi Lionel,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on drm-intel/for-linux-next]
[also build test WARNING on next-20180605]
[cannot apply to v4.17]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Lionel-Landwerlin/drm-i915-add-support-for-perf-configuration-queries/20180607-004646
base:   git://anongit.freedesktop.org/drm-intel for-linux-next
reproduce:
        # apt-get install sparse
        make ARCH=x86_64 allmodconfig
        make C=1 CF=-D__CHECK_ENDIAN__


sparse warnings: (new ones prefixed by >>)

>> drivers/gpu/drm/i915/i915_query.c:99:14: sparse: incorrect type in argument 1 (different base types) @@    expected void const volatile [noderef] <asn:1>*<noident> @@    got unsvoid const volatile [noderef] <asn:1>*<noident> @@
   drivers/gpu/drm/i915/i915_query.c:99:14:    expected void const volatile [noderef] <asn:1>*<noident>
   drivers/gpu/drm/i915/i915_query.c:99:14:    got unsigned long long [unsigned] [usertype] user_regs_ptr

vim +99 drivers/gpu/drm/i915/i915_query.c

    84	
    85	static int can_copy_perf_config_registers_or_number(u32 user_n_regs,
    86							    u64 user_regs_ptr,
    87							    u32 kernel_n_regs)
    88	{
    89		/*
    90		 * We'll just put the number of registers, and won't copy the
    91		 * register.
    92		 */
    93		if (user_n_regs == 0)
    94			return 0;
    95	
    96		if (user_n_regs < kernel_n_regs)
    97			return -EINVAL;
    98	
  > 99		if (!access_ok(VERIFY_WRITE, user_regs_ptr,
   100			       2 * sizeof(u32) * kernel_n_regs))
   101			return -EFAULT;
   102	
   103		return 0;
   104	}
   105	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index f8f9fd82a29a..e60385b77725 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1935,6 +1935,12 @@  struct drm_i915_private {
 		 */
 		struct idr metrics_idr;
 
+		/*
+		 * Number of dynamic configurations, you need to hold
+		 * dev_priv->perf.metrics_lock to access it.
+		 */
+		u32 n_metrics;
+
 		/*
 		 * Lock associated with anything below within this structure
 		 * except exclusive_stream.
diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
index 9f07900b872b..02f8034bb699 100644
--- a/drivers/gpu/drm/i915/i915_perf.c
+++ b/drivers/gpu/drm/i915/i915_perf.c
@@ -3365,6 +3365,8 @@  int i915_perf_add_config_ioctl(struct drm_device *dev, void *data,
 		goto sysfs_err;
 	}
 
+	dev_priv->perf.n_metrics++;
+
 	mutex_unlock(&dev_priv->perf.metrics_lock);
 
 	DRM_DEBUG("Added config %s id=%i\n", oa_config->uuid, oa_config->id);
@@ -3426,6 +3428,8 @@  int i915_perf_remove_config_ioctl(struct drm_device *dev, void *data,
 
 	idr_remove(&dev_priv->perf.metrics_idr, *arg);
 
+	dev_priv->perf.n_metrics--;
+
 	DRM_DEBUG("Removed config %s id=%i\n", oa_config->uuid, oa_config->id);
 
 	put_oa_config(dev_priv, oa_config);
diff --git a/drivers/gpu/drm/i915/i915_query.c b/drivers/gpu/drm/i915/i915_query.c
index 3f502eef2431..6583547de9df 100644
--- a/drivers/gpu/drm/i915/i915_query.c
+++ b/drivers/gpu/drm/i915/i915_query.c
@@ -84,9 +84,265 @@  static int query_topology_info(struct drm_i915_private *dev_priv,
 	return total_length;
 }
 
+static int can_copy_perf_config_registers_or_number(u32 user_n_regs,
+						    u64 user_regs_ptr,
+						    u32 kernel_n_regs)
+{
+	/*
+	 * We'll just put the number of registers, and won't copy the
+	 * register.
+	 */
+	if (user_n_regs == 0)
+		return 0;
+
+	if (user_n_regs < kernel_n_regs)
+		return -EINVAL;
+
+	if (!access_ok(VERIFY_WRITE, user_regs_ptr,
+		       2 * sizeof(u32) * kernel_n_regs))
+		return -EFAULT;
+
+	return 0;
+}
+
+static int copy_perf_config_registers_or_number(const struct i915_oa_reg *kernel_regs,
+						u32 kernel_n_regs,
+						u64 user_regs_ptr,
+						u32 *user_n_regs)
+{
+	u32 r;
+
+	if (*user_n_regs == 0) {
+		*user_n_regs = kernel_n_regs;
+		return 0;
+	}
+
+	*user_n_regs = kernel_n_regs;
+
+	for (r = 0; r < kernel_n_regs; r++) {
+		u32 __user *user_reg_ptr =
+			u64_to_user_ptr(user_regs_ptr + sizeof(u32) * r * 2);
+		u32 __user *user_val_ptr =
+			u64_to_user_ptr(user_regs_ptr + sizeof(u32) * r * 2 +
+					sizeof(u32));
+		int ret;
+
+		ret = __put_user(i915_mmio_reg_offset(kernel_regs[r].addr),
+				 user_reg_ptr);
+		if (ret)
+			return -EFAULT;
+
+		ret = __put_user(kernel_regs[r].value, user_val_ptr);
+		if (ret)
+			return -EFAULT;
+	}
+
+	return 0;
+}
+
+static int query_perf_config_data(struct drm_i915_private *i915,
+				  struct drm_i915_query_item *query_item)
+{
+	struct drm_i915_query_perf_config __user *user_query_config_ptr =
+		u64_to_user_ptr(query_item->data_ptr);
+	struct drm_i915_perf_oa_config __user *user_config_ptr =
+		u64_to_user_ptr(query_item->data_ptr +
+				sizeof(struct drm_i915_query_perf_config));
+	struct drm_i915_perf_oa_config user_config;
+	struct i915_oa_config *oa_config;
+	u64 config_id, flags;
+	u32 total_size;
+	int ret;
+
+	if (!i915->perf.initialized)
+		return -ENODEV;
+
+	total_size = sizeof(struct drm_i915_query_perf_config) +
+		sizeof(struct drm_i915_perf_oa_config);
+
+	if (query_item->length == 0)
+		return total_size;
+
+	if (query_item->length < total_size)
+		return -EINVAL;
+
+	if (!access_ok(VERIFY_WRITE, user_query_config_ptr, total_size))
+		return -EFAULT;
+
+	if (__get_user(flags, &user_query_config_ptr->flags))
+		return -EFAULT;
+
+	if (flags != 0)
+		return -EINVAL;
+
+	if (__get_user(config_id, &user_query_config_ptr->config))
+		return -EFAULT;
+
+	ret = mutex_lock_interruptible(&i915->perf.metrics_lock);
+	if (ret)
+		return ret;
+
+	if (config_id == 1) {
+		oa_config = &i915->perf.oa.test_config;
+	} else {
+		oa_config = idr_find(&i915->perf.metrics_idr, config_id);
+		if (!oa_config) {
+			ret = -ENOENT;
+			goto out;
+		}
+	}
+
+	if (__copy_from_user(&user_config, user_config_ptr,
+			     sizeof(user_config))) {
+		ret = -EFAULT;
+		goto out;
+	}
+
+	ret = can_copy_perf_config_registers_or_number(user_config.n_boolean_regs,
+						       user_config.boolean_regs_ptr,
+						       oa_config->b_counter_regs_len);
+	if (ret)
+		goto out;
+
+	ret = can_copy_perf_config_registers_or_number(user_config.n_flex_regs,
+						       user_config.flex_regs_ptr,
+						       oa_config->flex_regs_len);
+	if (ret)
+		goto out;
+
+	ret = can_copy_perf_config_registers_or_number(user_config.n_mux_regs,
+						       user_config.mux_regs_ptr,
+						       oa_config->mux_regs_len);
+	if (ret)
+		goto out;
+
+	ret = copy_perf_config_registers_or_number(oa_config->b_counter_regs,
+						   oa_config->b_counter_regs_len,
+						   user_config.boolean_regs_ptr,
+						   &user_config.n_boolean_regs);
+	if (ret)
+		goto out;
+
+	ret = copy_perf_config_registers_or_number(oa_config->flex_regs,
+						   oa_config->flex_regs_len,
+						   user_config.flex_regs_ptr,
+						   &user_config.n_flex_regs);
+	if (ret)
+		goto out;
+
+	ret = copy_perf_config_registers_or_number(oa_config->mux_regs,
+						   oa_config->mux_regs_len,
+						   user_config.mux_regs_ptr,
+						   &user_config.n_mux_regs);
+	if (ret)
+		goto out;
+
+	memcpy(user_config.uuid, oa_config->uuid, sizeof(user_config.uuid));
+
+	if (__copy_to_user(user_config_ptr, &user_config,
+			   sizeof(user_config))) {
+		ret = -EFAULT;
+		goto out;
+	}
+
+	ret = total_size;
+
+out:
+	mutex_unlock(&i915->perf.metrics_lock);
+	return ret;
+}
+
+static int query_perf_config_list(struct drm_i915_private *i915,
+				  struct drm_i915_query_item *query_item)
+{
+	struct drm_i915_query_perf_config __user *user_query_config_ptr =
+		u64_to_user_ptr(query_item->data_ptr);
+	struct i915_oa_config *oa_config;
+	u64 n_configs, flags;
+	u32 total_size;
+	int ret, id;
+
+	if (!i915->perf.initialized)
+		return -ENODEV;
+
+	/* Count the default test configuration */
+	n_configs = i915->perf.n_metrics + 1;
+	total_size = sizeof(struct drm_i915_query_perf_config) +
+		sizeof(u64) * n_configs;
+
+	if (query_item->length == 0) {
+		ret = total_size;
+		goto out;
+	}
+
+	if (query_item->length < total_size) {
+		ret = -EINVAL;
+		goto out;
+	}
+
+	if (!access_ok(VERIFY_WRITE, user_query_config_ptr, total_size)) {
+		ret = -EFAULT;
+		goto out;
+	}
+
+	if (__get_user(flags, &user_query_config_ptr->flags)) {
+		ret = -EFAULT;
+		goto out;
+	}
+
+	if (flags != 0) {
+		ret = -EINVAL;
+		goto out;
+	}
+
+	if (__put_user(n_configs, &user_query_config_ptr->config)) {
+		ret = -EFAULT;
+		goto out;
+	}
+
+	if (__put_user((u64) 1ULL, &user_query_config_ptr->data[0])) {
+		ret = -EFAULT;
+		goto out;
+	}
+
+	ret = mutex_lock_interruptible(&i915->perf.metrics_lock);
+	if (ret)
+		return ret;
+
+	n_configs = 1;
+	idr_for_each_entry(&i915->perf.metrics_idr, oa_config, id) {
+		if (__put_user((u64) id,
+			       &user_query_config_ptr->data[0] +
+			       n_configs * sizeof(u64))) {
+			ret = -EFAULT;
+			goto out;
+		}
+		n_configs++;
+	}
+
+	ret = total_size;
+
+out:
+	mutex_unlock(&i915->perf.metrics_lock);
+	return ret;
+}
+
+static int query_perf_config(struct drm_i915_private *i915,
+			     struct drm_i915_query_item *query_item)
+{
+	if (query_item->flags == DRM_I915_QUERY_PERF_CONFIG_LIST)
+		return query_perf_config_list(i915, query_item);
+	else if (query_item->flags == DRM_I915_QUERY_PERF_CONFIG_DATA)
+		return query_perf_config_data(i915, query_item);
+
+	return -EINVAL;
+}
+
+
 static int (* const i915_query_funcs[])(struct drm_i915_private *dev_priv,
 					struct drm_i915_query_item *query_item) = {
 	query_topology_info,
+	query_perf_config,
 };
 
 int i915_query_ioctl(struct drm_device *dev, void *data, struct drm_file *file)
diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index 21df158056a2..3d9c44f35df4 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -1663,6 +1663,7 @@  struct drm_i915_perf_oa_config {
 struct drm_i915_query_item {
 	__u64 query_id;
 #define DRM_I915_QUERY_TOPOLOGY_INFO    1
+#define DRM_I915_QUERY_PERF_CONFIG      2
 
 	/*
 	 * When set to zero by userspace, this is filled with the size of the
@@ -1673,9 +1674,16 @@  struct drm_i915_query_item {
 	__s32 length;
 
 	/*
-	 * Unused for now. Must be cleared to zero.
+	 * When query_id == DRM_I915_QUERY_TOPOLOGY_INFO, must be 0.
+	 *
+	 * When query_id == DRM_I915_QUERY_PERF_CONFIG, must be one of the
+	 * following :
+	 *         - DRM_I915_QUERY_PERF_CONFIG_LIST
+	 *         - DRM_I915_QUERY_PERF_CONFIG_DATA
 	 */
 	__u32 flags;
+#define DRM_I915_QUERY_PERF_CONFIG_LIST 1
+#define DRM_I915_QUERY_PERF_CONFIG_DATA 2
 
 	/*
 	 * Data will be written at the location pointed by data_ptr when the
@@ -1760,6 +1768,43 @@  struct drm_i915_query_topology_info {
 	__u8 data[];
 };
 
+/*
+ * Data written by the kernel with query DRM_I915_QUERY_PERF_CONFIG.
+ */
+struct drm_i915_query_perf_config {
+	/*
+	 *
+	 * When query_item.flags == DRM_I915_QUERY_PERF_CONFIG_LIST, i915 sets
+	 * this fields to the number of configurations available.
+	 *
+	 * When query_id == DRM_I915_QUERY_PERF_CONFIG_DATA, i915 will use the
+	 * value in this field as configuration identifier to decide what data
+	 * to write into config_ptr.
+	 */
+	__u64 config;
+
+	/*
+	 * Unused for now. Must be cleared to zero.
+	 */
+	__u64 flags;
+
+	/*
+	 * When query_item.flags == DRM_I915_QUERY_PERF_CONFIG_LIST, i915 will
+	 * write an array of __u64 of configuration identifiers.
+	 *
+	 * When query_item.flags == DRM_I915_QUERY_PERF_CONFIG_DATA, i915 will
+	 * write a struct drm_i915_perf_oa_config. If the following fields of
+	 * drm_i915_perf_oa_config are set not set to 0, i915 will write into
+	 * the associated pointers the values of submitted when the
+	 * configuration was created :
+	 *
+	 *         - n_mux_regs
+	 *         - n_boolean_regs
+	 *         - n_flex_regs
+	 */
+	__u8 data[];
+};
+
 #if defined(__cplusplus)
 }
 #endif