Re: [Intel-gfx] [PATCH v12 11/11] drm/i915: add support for perf configuration queries
Hi Lionel,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on drm-intel/for-linux-next]
[cannot apply to v5.3-rc7 next-20190902]
[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-Vulkan-performance-query-support/20190831-033234
base: git://anongit.freedesktop.org/drm-intel for-linux-next
If you fix the issue, kindly add following tag
Reported-by: kbuild test robot
Reported-by: Dan Carpenter
New smatch warnings:
drivers/gpu/drm/i915/i915_query.c:405 query_perf_config_list() warn: maybe
return -EFAULT instead of the bytes remaining?
Old smatch warnings:
drivers/gpu/drm/i915/i915_query.c:138 query_engine_info() warn: check that
'query.num_engines' doesn't leak information
#
https://github.com/0day-ci/linux/commit/1c566ee2d38f4e7ece15ee33fef205b1088e79bb
git remote add linux-review https://github.com/0day-ci/linux
git remote update linux-review
git checkout 1c566ee2d38f4e7ece15ee33fef205b1088e79bb
vim +405 drivers/gpu/drm/i915/i915_query.c
1c566ee2d38f4e Lionel Landwerlin 2019-08-30 336
1c566ee2d38f4e Lionel Landwerlin 2019-08-30 337 static int
query_perf_config_list(struct drm_i915_private *i915,
1c566ee2d38f4e Lionel Landwerlin 2019-08-30 338
struct drm_i915_query_item *query_item)
1c566ee2d38f4e Lionel Landwerlin 2019-08-30 339 {
1c566ee2d38f4e Lionel Landwerlin 2019-08-30 340struct
drm_i915_query_perf_config __user *user_query_config_ptr =
1c566ee2d38f4e Lionel Landwerlin 2019-08-30 341
u64_to_user_ptr(query_item->data_ptr);
1c566ee2d38f4e Lionel Landwerlin 2019-08-30 342struct i915_oa_config
*oa_config;
1c566ee2d38f4e Lionel Landwerlin 2019-08-30 343u32 flags, total_size;
1c566ee2d38f4e Lionel Landwerlin 2019-08-30 344u64 i, n_configs,
*oa_config_ids;
1c566ee2d38f4e Lionel Landwerlin 2019-08-30 345int ret, id;
1c566ee2d38f4e Lionel Landwerlin 2019-08-30 346
1c566ee2d38f4e Lionel Landwerlin 2019-08-30 347if
(!i915->perf.initialized)
1c566ee2d38f4e Lionel Landwerlin 2019-08-30 348return -ENODEV;
1c566ee2d38f4e Lionel Landwerlin 2019-08-30 349
1c566ee2d38f4e Lionel Landwerlin 2019-08-30 350/* Count the default
test configuration */
1c566ee2d38f4e Lionel Landwerlin 2019-08-30 351n_configs =
i915->perf.n_metrics + 1;
1c566ee2d38f4e Lionel Landwerlin 2019-08-30 352total_size =
sizeof(struct drm_i915_query_perf_config) +
1c566ee2d38f4e Lionel Landwerlin 2019-08-30 353sizeof(u64) *
n_configs;
1c566ee2d38f4e Lionel Landwerlin 2019-08-30 354
1c566ee2d38f4e Lionel Landwerlin 2019-08-30 355if (query_item->length
== 0)
1c566ee2d38f4e Lionel Landwerlin 2019-08-30 356return
total_size;
1c566ee2d38f4e Lionel Landwerlin 2019-08-30 357
1c566ee2d38f4e Lionel Landwerlin 2019-08-30 358if (query_item->length
< total_size) {
1c566ee2d38f4e Lionel Landwerlin 2019-08-30 359
DRM_DEBUG("Invalid query config list item size=%u expected=%u\n",
1c566ee2d38f4e Lionel Landwerlin 2019-08-30 360
query_item->length, total_size);
1c566ee2d38f4e Lionel Landwerlin 2019-08-30 361return -EINVAL;
1c566ee2d38f4e Lionel Landwerlin 2019-08-30 362}
1c566ee2d38f4e Lionel Landwerlin 2019-08-30 363
1c566ee2d38f4e Lionel Landwerlin 2019-08-30 364if
(!access_ok(user_query_config_ptr, total_size))
1c566ee2d38f4e Lionel Landwerlin 2019-08-30 365return -EFAULT;
1c566ee2d38f4e Lionel Landwerlin 2019-08-30 366
1c566ee2d38f4e Lionel Landwerlin 2019-08-30 367if (__get_user(flags,
&user_query_config_ptr->flags))
1c566ee2d38f4e Lionel Landwerlin 2019-08-30 368return -EFAULT;
1c566ee2d38f4e Lionel Landwerlin 2019-08-30 369
1c566ee2d38f4e Lionel Landwerlin 2019-08-30 370if (flags != 0)
1c566ee2d38f4e Lionel Landwerlin 2019-08-30 371return -EINVAL;
1c566ee2d38f4e Lionel Landwerlin 2019-08-30 372
1c566ee2d38f4e Lionel Landwerlin 2019-08-30 373if
(__put_user(n_configs, &user_query_config_ptr->config))
1c566ee2d38f4e Lionel Landwerlin 2019-08-30 374return -EFAULT;
1c566ee2d38f4e Lionel Landwerlin 2019-08-30 375
1c566ee2d38f4e Lionel Landwerlin 2019-08-30 376ret =
mutex_lock_interruptible(&i915->perf.metrics_lock);
1c566ee2d38f4e Lionel Landwerlin 2019-08-30 377if (ret)
1c566ee2d38f4e Lionel Landwerlin 2019-08-30 378return ret;
1c566ee2d38f4e Lionel Landwerlin 2019-08-30 379
1c566ee2d38f4e Lionel Landwerlin 2019-08-30 380/* Count the configs. */
1c566ee2d38f4e Lionel Landwerlin 2019-08-30 381n_configs = 1;
1c566ee2d38f4e Lionel Landwerlin 2019-08-30 382
idr_for_each_entry(&i915->perf.metrics_idr, oa_config, id)
1c566ee
[Intel-gfx] [PATCH v12 11/11] drm/i915: add support for perf configuration queries
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.
v2: Fix sparse warnings (Lionel)
Add support to query configuration using uuid (Lionel)
v3: Fix some inconsistency in uapi header (Lionel)
Fix unlocking when not locked issue (Lionel)
Add debug messages (Lionel)
v4: Fix missing unlock (Dan)
v5: Drop lock when copying config content to userspace (Chris)
v6: Drop lock when copying config list to userspace (Chris)
Fix deadlock when calling i915_perf_get_oa_config() under
perf.metrics_lock (Lionel)
Add i915_oa_config_get() (Chris)
Signed-off-by: Lionel Landwerlin
Reviewed-by: Chris Wilson
---
drivers/gpu/drm/i915/i915_drv.h | 6 +
drivers/gpu/drm/i915/i915_perf.c | 3 +
drivers/gpu/drm/i915/i915_query.c | 283 ++
include/uapi/drm/i915_drm.h | 65 ++-
4 files changed, 354 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index d18e12ada4e1..04d538dcc3c1 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1704,6 +1704,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 39681fb43034..54dba3487dfe 100644
--- a/drivers/gpu/drm/i915/i915_perf.c
+++ b/drivers/gpu/drm/i915/i915_perf.c
@@ -3910,6 +3910,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);
@@ -3970,6 +3972,7 @@ int i915_perf_remove_config_ioctl(struct drm_device *dev,
void *data,
&oa_config->sysfs_metric);
idr_remove(&dev_priv->perf.metrics_idr, *arg);
+ dev_priv->perf.n_metrics--;
mutex_unlock(&dev_priv->perf.metrics_lock);
diff --git a/drivers/gpu/drm/i915/i915_query.c
b/drivers/gpu/drm/i915/i915_query.c
index abac5042da2b..89b2821be4a0 100644
--- a/drivers/gpu/drm/i915/i915_query.c
+++ b/drivers/gpu/drm/i915/i915_query.c
@@ -7,6 +7,7 @@
#include
#include "i915_drv.h"
+#include "i915_perf.h"
#include "i915_query.h"
#include
@@ -140,10 +141,292 @@ query_engine_info(struct drm_i915_private *i915,
return len;
}
+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(u64_to_user_ptr(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,
+ b
