This replaces the manual read/write routines in debugfs with the common
simple attribute helpers. Doing this gets rid of repeated copy/pasting
of copy_from_user and value formatting code.
Signed-off-by: Kees Cook <[email protected]>
Cc: Daniel Vetter <[email protected]>
---
drivers/gpu/drm/i915/i915_debugfs.c | 394 +++++++++--------------------------
1 file changed, 98 insertions(+), 296 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index aae3148..d86c304 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -849,76 +849,42 @@ static const struct file_operations i915_error_state_fops = {
.release = i915_error_state_release,
};
-static ssize_t
-i915_next_seqno_read(struct file *filp,
- char __user *ubuf,
- size_t max,
- loff_t *ppos)
+static int
+i915_next_seqno_get(void *data, u64 *val)
{
- struct drm_device *dev = filp->private_data;
+ struct drm_device *dev = data;
drm_i915_private_t *dev_priv = dev->dev_private;
- char buf[80];
- int len;
int ret;
ret = mutex_lock_interruptible(&dev->struct_mutex);
if (ret)
return ret;
- len = snprintf(buf, sizeof(buf),
- "next_seqno : 0x%x\n",
- dev_priv->next_seqno);
-
+ *val = dev_priv->next_seqno;
mutex_unlock(&dev->struct_mutex);
- if (len > sizeof(buf))
- len = sizeof(buf);
-
- return simple_read_from_buffer(ubuf, max, ppos, buf, len);
+ return 0;
}
-static ssize_t
-i915_next_seqno_write(struct file *filp,
- const char __user *ubuf,
- size_t cnt,
- loff_t *ppos)
-{
- struct drm_device *dev = filp->private_data;
- char buf[20];
- u32 val = 1;
+static int
+i915_next_seqno_set(void *data, u64 val)
+{
+ struct drm_device *dev = data;
int ret;
- if (cnt > 0) {
- if (cnt > sizeof(buf) - 1)
- return -EINVAL;
-
- if (copy_from_user(buf, ubuf, cnt))
- return -EFAULT;
- buf[cnt] = 0;
-
- ret = kstrtouint(buf, 0, &val);
- if (ret < 0)
- return ret;
- }
-
ret = mutex_lock_interruptible(&dev->struct_mutex);
if (ret)
return ret;
ret = i915_gem_set_seqno(dev, val);
-
mutex_unlock(&dev->struct_mutex);
- return ret ?: cnt;
+ return ret;
}
-static const struct file_operations i915_next_seqno_fops = {
- .owner = THIS_MODULE,
- .open = simple_open,
- .read = i915_next_seqno_read,
- .write = i915_next_seqno_write,
- .llseek = default_llseek,
-};
+DEFINE_SIMPLE_ATTRIBUTE(i915_next_seqno_fops,
+ i915_next_seqno_get, i915_next_seqno_set,
+ "next_seqno : 0x%llx\n");
static int i915_rstdby_delays(struct seq_file *m, void *unused)
{
@@ -1680,105 +1646,51 @@ static int i915_dpio_info(struct seq_file *m, void *data)
return 0;
}
-static ssize_t
-i915_wedged_read(struct file *filp,
- char __user *ubuf,
- size_t max,
- loff_t *ppos)
+static int
+i915_wedged_get(void *data, u64 *val)
{
- struct drm_device *dev = filp->private_data;
+ struct drm_device *dev = data;
drm_i915_private_t *dev_priv = dev->dev_private;
- char buf[80];
- int len;
- len = snprintf(buf, sizeof(buf),
- "wedged : %d\n",
- atomic_read(&dev_priv->gpu_error.reset_counter));
+ *val = atomic_read(&dev_priv->gpu_error.reset_counter);
- if (len > sizeof(buf))
- len = sizeof(buf);
-
- return simple_read_from_buffer(ubuf, max, ppos, buf, len);
+ return 0;
}
-static ssize_t
-i915_wedged_write(struct file *filp,
- const char __user *ubuf,
- size_t cnt,
- loff_t *ppos)
+static int
+i915_wedged_set(void *data, u64 val)
{
- struct drm_device *dev = filp->private_data;
- char buf[20];
- int val = 1;
-
- if (cnt > 0) {
- if (cnt > sizeof(buf) - 1)
- return -EINVAL;
+ struct drm_device *dev = data;
- if (copy_from_user(buf, ubuf, cnt))
- return -EFAULT;
- buf[cnt] = 0;
-
- val = simple_strtoul(buf, NULL, 0);
- }
-
- DRM_INFO("Manually setting wedged to %d\n", val);
+ DRM_INFO("Manually setting wedged to %llu\n", val);
i915_handle_error(dev, val);
- return cnt;
+ return 0;
}
-static const struct file_operations i915_wedged_fops = {
- .owner = THIS_MODULE,
- .open = simple_open,
- .read = i915_wedged_read,
- .write = i915_wedged_write,
- .llseek = default_llseek,
-};
+DEFINE_SIMPLE_ATTRIBUTE(i915_wedged_fops,
+ i915_wedged_get, i915_wedged_set,
+ "wedged : %llu\n");
-static ssize_t
-i915_ring_stop_read(struct file *filp,
- char __user *ubuf,
- size_t max,
- loff_t *ppos)
+static int
+i915_ring_stop_get(void *data, u64 *val)
{
- struct drm_device *dev = filp->private_data;
+ struct drm_device *dev = data;
drm_i915_private_t *dev_priv = dev->dev_private;
- char buf[20];
- int len;
-
- len = snprintf(buf, sizeof(buf),
- "0x%08x\n", dev_priv->gpu_error.stop_rings);
- if (len > sizeof(buf))
- len = sizeof(buf);
+ *val = dev_priv->gpu_error.stop_rings;
- return simple_read_from_buffer(ubuf, max, ppos, buf, len);
+ return 0;
}
-static ssize_t
-i915_ring_stop_write(struct file *filp,
- const char __user *ubuf,
- size_t cnt,
- loff_t *ppos)
+static int
+i915_ring_stop_set(void *data, u64 val)
{
- struct drm_device *dev = filp->private_data;
+ struct drm_device *dev = data;
struct drm_i915_private *dev_priv = dev->dev_private;
- char buf[20];
- int val = 0, ret;
-
- if (cnt > 0) {
- if (cnt > sizeof(buf) - 1)
- return -EINVAL;
-
- if (copy_from_user(buf, ubuf, cnt))
- return -EFAULT;
- buf[cnt] = 0;
-
- val = simple_strtoul(buf, NULL, 0);
- }
+ int ret;
- DRM_DEBUG_DRIVER("Stopping rings 0x%08x\n", val);
+ DRM_DEBUG_DRIVER("Stopping rings 0x%08llx\n", val);
ret = mutex_lock_interruptible(&dev->struct_mutex);
if (ret)
@@ -1787,16 +1699,12 @@ i915_ring_stop_write(struct file *filp,
dev_priv->gpu_error.stop_rings = val;
mutex_unlock(&dev->struct_mutex);
- return cnt;
+ return 0;
}
-static const struct file_operations i915_ring_stop_fops = {
- .owner = THIS_MODULE,
- .open = simple_open,
- .read = i915_ring_stop_read,
- .write = i915_ring_stop_write,
- .llseek = default_llseek,
-};
+DEFINE_SIMPLE_ATTRIBUTE(i915_ring_stop_fops,
+ i915_ring_stop_get, i915_ring_stop_set,
+ "0x%08llx\n");
#define DROP_UNBOUND 0x1
#define DROP_BOUND 0x2
@@ -1806,46 +1714,23 @@ static const struct file_operations i915_ring_stop_fops = {
DROP_BOUND | \
DROP_RETIRE | \
DROP_ACTIVE)
-static ssize_t
-i915_drop_caches_read(struct file *filp,
- char __user *ubuf,
- size_t max,
- loff_t *ppos)
+static int
+i915_drop_caches_get(void *data, u64 *val)
{
- char buf[20];
- int len;
-
- len = snprintf(buf, sizeof(buf), "0x%08x\n", DROP_ALL);
- if (len > sizeof(buf))
- len = sizeof(buf);
+ *val = DROP_ALL;
- return simple_read_from_buffer(ubuf, max, ppos, buf, len);
+ return 0;
}
-static ssize_t
-i915_drop_caches_write(struct file *filp,
- const char __user *ubuf,
- size_t cnt,
- loff_t *ppos)
+static int
+i915_drop_caches_set(void *data, u64 val)
{
- struct drm_device *dev = filp->private_data;
+ struct drm_device *dev = data;
struct drm_i915_private *dev_priv = dev->dev_private;
struct drm_i915_gem_object *obj, *next;
- char buf[20];
- int val = 0, ret;
-
- if (cnt > 0) {
- if (cnt > sizeof(buf) - 1)
- return -EINVAL;
-
- if (copy_from_user(buf, ubuf, cnt))
- return -EFAULT;
- buf[cnt] = 0;
-
- val = simple_strtoul(buf, NULL, 0);
- }
+ int ret;
- DRM_DEBUG_DRIVER("Dropping caches: 0x%08x\n", val);
+ DRM_DEBUG_DRIVER("Dropping caches: 0x%08llx\n", val);
/* No need to check and wait for gpu resets, only libdrm auto-restarts
* on ioctls on -EAGAIN. */
@@ -1883,27 +1768,19 @@ i915_drop_caches_write(struct file *filp,
unlock:
mutex_unlock(&dev->struct_mutex);
- return ret ?: cnt;
+ return ret;
}
-static const struct file_operations i915_drop_caches_fops = {
- .owner = THIS_MODULE,
- .open = simple_open,
- .read = i915_drop_caches_read,
- .write = i915_drop_caches_write,
- .llseek = default_llseek,
-};
+DEFINE_SIMPLE_ATTRIBUTE(i915_drop_caches_fops,
+ i915_drop_caches_get, i915_drop_caches_set,
+ "0x%08llx\n");
-static ssize_t
-i915_max_freq_read(struct file *filp,
- char __user *ubuf,
- size_t max,
- loff_t *ppos)
+static int
+i915_max_freq_get(void *data, u64 *val)
{
- struct drm_device *dev = filp->private_data;
+ struct drm_device *dev = data;
drm_i915_private_t *dev_priv = dev->dev_private;
- char buf[80];
- int len, ret;
+ int ret;
if (!(IS_GEN6(dev) || IS_GEN7(dev)))
return -ENODEV;
@@ -1912,42 +1789,23 @@ i915_max_freq_read(struct file *filp,
if (ret)
return ret;
- len = snprintf(buf, sizeof(buf),
- "max freq: %d\n", dev_priv->rps.max_delay * GT_FREQUENCY_MULTIPLIER);
+ *val = dev_priv->rps.max_delay * GT_FREQUENCY_MULTIPLIER;
mutex_unlock(&dev_priv->rps.hw_lock);
- if (len > sizeof(buf))
- len = sizeof(buf);
-
- return simple_read_from_buffer(ubuf, max, ppos, buf, len);
+ return 0;
}
-static ssize_t
-i915_max_freq_write(struct file *filp,
- const char __user *ubuf,
- size_t cnt,
- loff_t *ppos)
+static int
+i915_max_freq_set(void *data, u64 val)
{
- struct drm_device *dev = filp->private_data;
+ struct drm_device *dev = data;
struct drm_i915_private *dev_priv = dev->dev_private;
- char buf[20];
- int val = 1, ret;
+ int ret;
if (!(IS_GEN6(dev) || IS_GEN7(dev)))
return -ENODEV;
- if (cnt > 0) {
- if (cnt > sizeof(buf) - 1)
- return -EINVAL;
-
- if (copy_from_user(buf, ubuf, cnt))
- return -EFAULT;
- buf[cnt] = 0;
-
- val = simple_strtoul(buf, NULL, 0);
- }
-
- DRM_DEBUG_DRIVER("Manually setting max freq to %d\n", val);
+ DRM_DEBUG_DRIVER("Manually setting max freq to %llu\n", val);
ret = mutex_lock_interruptible(&dev_priv->rps.hw_lock);
if (ret)
@@ -1961,25 +1819,19 @@ i915_max_freq_write(struct file *filp,
gen6_set_rps(dev, val / GT_FREQUENCY_MULTIPLIER);
mutex_unlock(&dev_priv->rps.hw_lock);
- return cnt;
+ return 0;
}
-static const struct file_operations i915_max_freq_fops = {
- .owner = THIS_MODULE,
- .open = simple_open,
- .read = i915_max_freq_read,
- .write = i915_max_freq_write,
- .llseek = default_llseek,
-};
+DEFINE_SIMPLE_ATTRIBUTE(i915_max_freq_fops,
+ i915_max_freq_get, i915_max_freq_set,
+ "max freq: %llu\n");
-static ssize_t
-i915_min_freq_read(struct file *filp, char __user *ubuf, size_t max,
- loff_t *ppos)
+static int
+i915_min_freq_get(void *data, u64 *val)
{
- struct drm_device *dev = filp->private_data;
+ struct drm_device *dev = data;
drm_i915_private_t *dev_priv = dev->dev_private;
- char buf[80];
- int len, ret;
+ int ret;
if (!(IS_GEN6(dev) || IS_GEN7(dev)))
return -ENODEV;
@@ -1988,40 +1840,23 @@ i915_min_freq_read(struct file *filp, char __user *ubuf, size_t max,
if (ret)
return ret;
- len = snprintf(buf, sizeof(buf),
- "min freq: %d\n", dev_priv->rps.min_delay * GT_FREQUENCY_MULTIPLIER);
+ *val = dev_priv->rps.min_delay * GT_FREQUENCY_MULTIPLIER;
mutex_unlock(&dev_priv->rps.hw_lock);
- if (len > sizeof(buf))
- len = sizeof(buf);
-
- return simple_read_from_buffer(ubuf, max, ppos, buf, len);
+ return 0;
}
-static ssize_t
-i915_min_freq_write(struct file *filp, const char __user *ubuf, size_t cnt,
- loff_t *ppos)
+static int
+i915_min_freq_set(void *data, u64 val)
{
- struct drm_device *dev = filp->private_data;
+ struct drm_device *dev = data;
struct drm_i915_private *dev_priv = dev->dev_private;
- char buf[20];
- int val = 1, ret;
+ int ret;
if (!(IS_GEN6(dev) || IS_GEN7(dev)))
return -ENODEV;
- if (cnt > 0) {
- if (cnt > sizeof(buf) - 1)
- return -EINVAL;
-
- if (copy_from_user(buf, ubuf, cnt))
- return -EFAULT;
- buf[cnt] = 0;
-
- val = simple_strtoul(buf, NULL, 0);
- }
-
- DRM_DEBUG_DRIVER("Manually setting min freq to %d\n", val);
+ DRM_DEBUG_DRIVER("Manually setting min freq to %llu\n", val);
ret = mutex_lock_interruptible(&dev_priv->rps.hw_lock);
if (ret)
@@ -2035,28 +1870,20 @@ i915_min_freq_write(struct file *filp, const char __user *ubuf, size_t cnt,
gen6_set_rps(dev, val / GT_FREQUENCY_MULTIPLIER);
mutex_unlock(&dev_priv->rps.hw_lock);
- return cnt;
+ return 0;
}
-static const struct file_operations i915_min_freq_fops = {
- .owner = THIS_MODULE,
- .open = simple_open,
- .read = i915_min_freq_read,
- .write = i915_min_freq_write,
- .llseek = default_llseek,
-};
+DEFINE_SIMPLE_ATTRIBUTE(i915_min_freq_fops,
+ i915_min_freq_get, i915_min_freq_set,
+ "min freq: %llu\n");
-static ssize_t
-i915_cache_sharing_read(struct file *filp,
- char __user *ubuf,
- size_t max,
- loff_t *ppos)
+static int
+i915_cache_sharing_get(void *data, u64 *val)
{
- struct drm_device *dev = filp->private_data;
+ struct drm_device *dev = data;
drm_i915_private_t *dev_priv = dev->dev_private;
- char buf[80];
u32 snpcr;
- int len, ret;
+ int ret;
if (!(IS_GEN6(dev) || IS_GEN7(dev)))
return -ENODEV;
@@ -2068,46 +1895,25 @@ i915_cache_sharing_read(struct file *filp,
snpcr = I915_READ(GEN6_MBCUNIT_SNPCR);
mutex_unlock(&dev_priv->dev->struct_mutex);
- len = snprintf(buf, sizeof(buf),
- "%d\n", (snpcr & GEN6_MBC_SNPCR_MASK) >>
- GEN6_MBC_SNPCR_SHIFT);
-
- if (len > sizeof(buf))
- len = sizeof(buf);
+ *val = (snpcr & GEN6_MBC_SNPCR_MASK) >> GEN6_MBC_SNPCR_SHIFT;
- return simple_read_from_buffer(ubuf, max, ppos, buf, len);
+ return 0;
}
-static ssize_t
-i915_cache_sharing_write(struct file *filp,
- const char __user *ubuf,
- size_t cnt,
- loff_t *ppos)
+static int
+i915_cache_sharing_set(void *data, u64 val)
{
- struct drm_device *dev = filp->private_data;
+ struct drm_device *dev = data;
struct drm_i915_private *dev_priv = dev->dev_private;
- char buf[20];
u32 snpcr;
- int val = 1;
if (!(IS_GEN6(dev) || IS_GEN7(dev)))
return -ENODEV;
- if (cnt > 0) {
- if (cnt > sizeof(buf) - 1)
- return -EINVAL;
-
- if (copy_from_user(buf, ubuf, cnt))
- return -EFAULT;
- buf[cnt] = 0;
-
- val = simple_strtoul(buf, NULL, 0);
- }
-
- if (val < 0 || val > 3)
+ if (val > 3)
return -EINVAL;
- DRM_DEBUG_DRIVER("Manually setting uncore sharing to %d\n", val);
+ DRM_DEBUG_DRIVER("Manually setting uncore sharing to %llu\n", val);
/* Update the cache sharing policy here as well */
snpcr = I915_READ(GEN6_MBCUNIT_SNPCR);
@@ -2115,16 +1921,12 @@ i915_cache_sharing_write(struct file *filp,
snpcr |= (val << GEN6_MBC_SNPCR_SHIFT);
I915_WRITE(GEN6_MBCUNIT_SNPCR, snpcr);
- return cnt;
+ return 0;
}
-static const struct file_operations i915_cache_sharing_fops = {
- .owner = THIS_MODULE,
- .open = simple_open,
- .read = i915_cache_sharing_read,
- .write = i915_cache_sharing_write,
- .llseek = default_llseek,
-};
+DEFINE_SIMPLE_ATTRIBUTE(i915_cache_sharing_fops,
+ i915_cache_sharing_get, i915_cache_sharing_set,
+ "%llu\n");
/* As the drm_debugfs_init() routines are called before dev->dev_private is
* allocated we need to hook into the minor for release. */
--
1.7.9.5
--
Kees Cook
Chrome OS Security
On Sun, Mar 10, 2013 at 02:10:06PM -0700, Kees Cook wrote:
> This replaces the manual read/write routines in debugfs with the common
> simple attribute helpers. Doing this gets rid of repeated copy/pasting
> of copy_from_user and value formatting code.
>
> Signed-off-by: Kees Cook <[email protected]>
> Cc: Daniel Vetter <[email protected]>
Queued for -next, thanks for the patch. When sending drm/i915 patches,
please also cc [email protected] (it's open for
non-subscribers non without nagging "you're mail is in the moderation
queue" replies).
Thanks, Daniel
> ---
> drivers/gpu/drm/i915/i915_debugfs.c | 394 +++++++++--------------------------
> 1 file changed, 98 insertions(+), 296 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index aae3148..d86c304 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -849,76 +849,42 @@ static const struct file_operations i915_error_state_fops = {
> .release = i915_error_state_release,
> };
>
> -static ssize_t
> -i915_next_seqno_read(struct file *filp,
> - char __user *ubuf,
> - size_t max,
> - loff_t *ppos)
> +static int
> +i915_next_seqno_get(void *data, u64 *val)
> {
> - struct drm_device *dev = filp->private_data;
> + struct drm_device *dev = data;
> drm_i915_private_t *dev_priv = dev->dev_private;
> - char buf[80];
> - int len;
> int ret;
>
> ret = mutex_lock_interruptible(&dev->struct_mutex);
> if (ret)
> return ret;
>
> - len = snprintf(buf, sizeof(buf),
> - "next_seqno : 0x%x\n",
> - dev_priv->next_seqno);
> -
> + *val = dev_priv->next_seqno;
> mutex_unlock(&dev->struct_mutex);
>
> - if (len > sizeof(buf))
> - len = sizeof(buf);
> -
> - return simple_read_from_buffer(ubuf, max, ppos, buf, len);
> + return 0;
> }
>
> -static ssize_t
> -i915_next_seqno_write(struct file *filp,
> - const char __user *ubuf,
> - size_t cnt,
> - loff_t *ppos)
> -{
> - struct drm_device *dev = filp->private_data;
> - char buf[20];
> - u32 val = 1;
> +static int
> +i915_next_seqno_set(void *data, u64 val)
> +{
> + struct drm_device *dev = data;
> int ret;
>
> - if (cnt > 0) {
> - if (cnt > sizeof(buf) - 1)
> - return -EINVAL;
> -
> - if (copy_from_user(buf, ubuf, cnt))
> - return -EFAULT;
> - buf[cnt] = 0;
> -
> - ret = kstrtouint(buf, 0, &val);
> - if (ret < 0)
> - return ret;
> - }
> -
> ret = mutex_lock_interruptible(&dev->struct_mutex);
> if (ret)
> return ret;
>
> ret = i915_gem_set_seqno(dev, val);
> -
> mutex_unlock(&dev->struct_mutex);
>
> - return ret ?: cnt;
> + return ret;
> }
>
> -static const struct file_operations i915_next_seqno_fops = {
> - .owner = THIS_MODULE,
> - .open = simple_open,
> - .read = i915_next_seqno_read,
> - .write = i915_next_seqno_write,
> - .llseek = default_llseek,
> -};
> +DEFINE_SIMPLE_ATTRIBUTE(i915_next_seqno_fops,
> + i915_next_seqno_get, i915_next_seqno_set,
> + "next_seqno : 0x%llx\n");
>
> static int i915_rstdby_delays(struct seq_file *m, void *unused)
> {
> @@ -1680,105 +1646,51 @@ static int i915_dpio_info(struct seq_file *m, void *data)
> return 0;
> }
>
> -static ssize_t
> -i915_wedged_read(struct file *filp,
> - char __user *ubuf,
> - size_t max,
> - loff_t *ppos)
> +static int
> +i915_wedged_get(void *data, u64 *val)
> {
> - struct drm_device *dev = filp->private_data;
> + struct drm_device *dev = data;
> drm_i915_private_t *dev_priv = dev->dev_private;
> - char buf[80];
> - int len;
>
> - len = snprintf(buf, sizeof(buf),
> - "wedged : %d\n",
> - atomic_read(&dev_priv->gpu_error.reset_counter));
> + *val = atomic_read(&dev_priv->gpu_error.reset_counter);
>
> - if (len > sizeof(buf))
> - len = sizeof(buf);
> -
> - return simple_read_from_buffer(ubuf, max, ppos, buf, len);
> + return 0;
> }
>
> -static ssize_t
> -i915_wedged_write(struct file *filp,
> - const char __user *ubuf,
> - size_t cnt,
> - loff_t *ppos)
> +static int
> +i915_wedged_set(void *data, u64 val)
> {
> - struct drm_device *dev = filp->private_data;
> - char buf[20];
> - int val = 1;
> -
> - if (cnt > 0) {
> - if (cnt > sizeof(buf) - 1)
> - return -EINVAL;
> + struct drm_device *dev = data;
>
> - if (copy_from_user(buf, ubuf, cnt))
> - return -EFAULT;
> - buf[cnt] = 0;
> -
> - val = simple_strtoul(buf, NULL, 0);
> - }
> -
> - DRM_INFO("Manually setting wedged to %d\n", val);
> + DRM_INFO("Manually setting wedged to %llu\n", val);
> i915_handle_error(dev, val);
>
> - return cnt;
> + return 0;
> }
>
> -static const struct file_operations i915_wedged_fops = {
> - .owner = THIS_MODULE,
> - .open = simple_open,
> - .read = i915_wedged_read,
> - .write = i915_wedged_write,
> - .llseek = default_llseek,
> -};
> +DEFINE_SIMPLE_ATTRIBUTE(i915_wedged_fops,
> + i915_wedged_get, i915_wedged_set,
> + "wedged : %llu\n");
>
> -static ssize_t
> -i915_ring_stop_read(struct file *filp,
> - char __user *ubuf,
> - size_t max,
> - loff_t *ppos)
> +static int
> +i915_ring_stop_get(void *data, u64 *val)
> {
> - struct drm_device *dev = filp->private_data;
> + struct drm_device *dev = data;
> drm_i915_private_t *dev_priv = dev->dev_private;
> - char buf[20];
> - int len;
> -
> - len = snprintf(buf, sizeof(buf),
> - "0x%08x\n", dev_priv->gpu_error.stop_rings);
>
> - if (len > sizeof(buf))
> - len = sizeof(buf);
> + *val = dev_priv->gpu_error.stop_rings;
>
> - return simple_read_from_buffer(ubuf, max, ppos, buf, len);
> + return 0;
> }
>
> -static ssize_t
> -i915_ring_stop_write(struct file *filp,
> - const char __user *ubuf,
> - size_t cnt,
> - loff_t *ppos)
> +static int
> +i915_ring_stop_set(void *data, u64 val)
> {
> - struct drm_device *dev = filp->private_data;
> + struct drm_device *dev = data;
> struct drm_i915_private *dev_priv = dev->dev_private;
> - char buf[20];
> - int val = 0, ret;
> -
> - if (cnt > 0) {
> - if (cnt > sizeof(buf) - 1)
> - return -EINVAL;
> -
> - if (copy_from_user(buf, ubuf, cnt))
> - return -EFAULT;
> - buf[cnt] = 0;
> -
> - val = simple_strtoul(buf, NULL, 0);
> - }
> + int ret;
>
> - DRM_DEBUG_DRIVER("Stopping rings 0x%08x\n", val);
> + DRM_DEBUG_DRIVER("Stopping rings 0x%08llx\n", val);
>
> ret = mutex_lock_interruptible(&dev->struct_mutex);
> if (ret)
> @@ -1787,16 +1699,12 @@ i915_ring_stop_write(struct file *filp,
> dev_priv->gpu_error.stop_rings = val;
> mutex_unlock(&dev->struct_mutex);
>
> - return cnt;
> + return 0;
> }
>
> -static const struct file_operations i915_ring_stop_fops = {
> - .owner = THIS_MODULE,
> - .open = simple_open,
> - .read = i915_ring_stop_read,
> - .write = i915_ring_stop_write,
> - .llseek = default_llseek,
> -};
> +DEFINE_SIMPLE_ATTRIBUTE(i915_ring_stop_fops,
> + i915_ring_stop_get, i915_ring_stop_set,
> + "0x%08llx\n");
>
> #define DROP_UNBOUND 0x1
> #define DROP_BOUND 0x2
> @@ -1806,46 +1714,23 @@ static const struct file_operations i915_ring_stop_fops = {
> DROP_BOUND | \
> DROP_RETIRE | \
> DROP_ACTIVE)
> -static ssize_t
> -i915_drop_caches_read(struct file *filp,
> - char __user *ubuf,
> - size_t max,
> - loff_t *ppos)
> +static int
> +i915_drop_caches_get(void *data, u64 *val)
> {
> - char buf[20];
> - int len;
> -
> - len = snprintf(buf, sizeof(buf), "0x%08x\n", DROP_ALL);
> - if (len > sizeof(buf))
> - len = sizeof(buf);
> + *val = DROP_ALL;
>
> - return simple_read_from_buffer(ubuf, max, ppos, buf, len);
> + return 0;
> }
>
> -static ssize_t
> -i915_drop_caches_write(struct file *filp,
> - const char __user *ubuf,
> - size_t cnt,
> - loff_t *ppos)
> +static int
> +i915_drop_caches_set(void *data, u64 val)
> {
> - struct drm_device *dev = filp->private_data;
> + struct drm_device *dev = data;
> struct drm_i915_private *dev_priv = dev->dev_private;
> struct drm_i915_gem_object *obj, *next;
> - char buf[20];
> - int val = 0, ret;
> -
> - if (cnt > 0) {
> - if (cnt > sizeof(buf) - 1)
> - return -EINVAL;
> -
> - if (copy_from_user(buf, ubuf, cnt))
> - return -EFAULT;
> - buf[cnt] = 0;
> -
> - val = simple_strtoul(buf, NULL, 0);
> - }
> + int ret;
>
> - DRM_DEBUG_DRIVER("Dropping caches: 0x%08x\n", val);
> + DRM_DEBUG_DRIVER("Dropping caches: 0x%08llx\n", val);
>
> /* No need to check and wait for gpu resets, only libdrm auto-restarts
> * on ioctls on -EAGAIN. */
> @@ -1883,27 +1768,19 @@ i915_drop_caches_write(struct file *filp,
> unlock:
> mutex_unlock(&dev->struct_mutex);
>
> - return ret ?: cnt;
> + return ret;
> }
>
> -static const struct file_operations i915_drop_caches_fops = {
> - .owner = THIS_MODULE,
> - .open = simple_open,
> - .read = i915_drop_caches_read,
> - .write = i915_drop_caches_write,
> - .llseek = default_llseek,
> -};
> +DEFINE_SIMPLE_ATTRIBUTE(i915_drop_caches_fops,
> + i915_drop_caches_get, i915_drop_caches_set,
> + "0x%08llx\n");
>
> -static ssize_t
> -i915_max_freq_read(struct file *filp,
> - char __user *ubuf,
> - size_t max,
> - loff_t *ppos)
> +static int
> +i915_max_freq_get(void *data, u64 *val)
> {
> - struct drm_device *dev = filp->private_data;
> + struct drm_device *dev = data;
> drm_i915_private_t *dev_priv = dev->dev_private;
> - char buf[80];
> - int len, ret;
> + int ret;
>
> if (!(IS_GEN6(dev) || IS_GEN7(dev)))
> return -ENODEV;
> @@ -1912,42 +1789,23 @@ i915_max_freq_read(struct file *filp,
> if (ret)
> return ret;
>
> - len = snprintf(buf, sizeof(buf),
> - "max freq: %d\n", dev_priv->rps.max_delay * GT_FREQUENCY_MULTIPLIER);
> + *val = dev_priv->rps.max_delay * GT_FREQUENCY_MULTIPLIER;
> mutex_unlock(&dev_priv->rps.hw_lock);
>
> - if (len > sizeof(buf))
> - len = sizeof(buf);
> -
> - return simple_read_from_buffer(ubuf, max, ppos, buf, len);
> + return 0;
> }
>
> -static ssize_t
> -i915_max_freq_write(struct file *filp,
> - const char __user *ubuf,
> - size_t cnt,
> - loff_t *ppos)
> +static int
> +i915_max_freq_set(void *data, u64 val)
> {
> - struct drm_device *dev = filp->private_data;
> + struct drm_device *dev = data;
> struct drm_i915_private *dev_priv = dev->dev_private;
> - char buf[20];
> - int val = 1, ret;
> + int ret;
>
> if (!(IS_GEN6(dev) || IS_GEN7(dev)))
> return -ENODEV;
>
> - if (cnt > 0) {
> - if (cnt > sizeof(buf) - 1)
> - return -EINVAL;
> -
> - if (copy_from_user(buf, ubuf, cnt))
> - return -EFAULT;
> - buf[cnt] = 0;
> -
> - val = simple_strtoul(buf, NULL, 0);
> - }
> -
> - DRM_DEBUG_DRIVER("Manually setting max freq to %d\n", val);
> + DRM_DEBUG_DRIVER("Manually setting max freq to %llu\n", val);
>
> ret = mutex_lock_interruptible(&dev_priv->rps.hw_lock);
> if (ret)
> @@ -1961,25 +1819,19 @@ i915_max_freq_write(struct file *filp,
> gen6_set_rps(dev, val / GT_FREQUENCY_MULTIPLIER);
> mutex_unlock(&dev_priv->rps.hw_lock);
>
> - return cnt;
> + return 0;
> }
>
> -static const struct file_operations i915_max_freq_fops = {
> - .owner = THIS_MODULE,
> - .open = simple_open,
> - .read = i915_max_freq_read,
> - .write = i915_max_freq_write,
> - .llseek = default_llseek,
> -};
> +DEFINE_SIMPLE_ATTRIBUTE(i915_max_freq_fops,
> + i915_max_freq_get, i915_max_freq_set,
> + "max freq: %llu\n");
>
> -static ssize_t
> -i915_min_freq_read(struct file *filp, char __user *ubuf, size_t max,
> - loff_t *ppos)
> +static int
> +i915_min_freq_get(void *data, u64 *val)
> {
> - struct drm_device *dev = filp->private_data;
> + struct drm_device *dev = data;
> drm_i915_private_t *dev_priv = dev->dev_private;
> - char buf[80];
> - int len, ret;
> + int ret;
>
> if (!(IS_GEN6(dev) || IS_GEN7(dev)))
> return -ENODEV;
> @@ -1988,40 +1840,23 @@ i915_min_freq_read(struct file *filp, char __user *ubuf, size_t max,
> if (ret)
> return ret;
>
> - len = snprintf(buf, sizeof(buf),
> - "min freq: %d\n", dev_priv->rps.min_delay * GT_FREQUENCY_MULTIPLIER);
> + *val = dev_priv->rps.min_delay * GT_FREQUENCY_MULTIPLIER;
> mutex_unlock(&dev_priv->rps.hw_lock);
>
> - if (len > sizeof(buf))
> - len = sizeof(buf);
> -
> - return simple_read_from_buffer(ubuf, max, ppos, buf, len);
> + return 0;
> }
>
> -static ssize_t
> -i915_min_freq_write(struct file *filp, const char __user *ubuf, size_t cnt,
> - loff_t *ppos)
> +static int
> +i915_min_freq_set(void *data, u64 val)
> {
> - struct drm_device *dev = filp->private_data;
> + struct drm_device *dev = data;
> struct drm_i915_private *dev_priv = dev->dev_private;
> - char buf[20];
> - int val = 1, ret;
> + int ret;
>
> if (!(IS_GEN6(dev) || IS_GEN7(dev)))
> return -ENODEV;
>
> - if (cnt > 0) {
> - if (cnt > sizeof(buf) - 1)
> - return -EINVAL;
> -
> - if (copy_from_user(buf, ubuf, cnt))
> - return -EFAULT;
> - buf[cnt] = 0;
> -
> - val = simple_strtoul(buf, NULL, 0);
> - }
> -
> - DRM_DEBUG_DRIVER("Manually setting min freq to %d\n", val);
> + DRM_DEBUG_DRIVER("Manually setting min freq to %llu\n", val);
>
> ret = mutex_lock_interruptible(&dev_priv->rps.hw_lock);
> if (ret)
> @@ -2035,28 +1870,20 @@ i915_min_freq_write(struct file *filp, const char __user *ubuf, size_t cnt,
> gen6_set_rps(dev, val / GT_FREQUENCY_MULTIPLIER);
> mutex_unlock(&dev_priv->rps.hw_lock);
>
> - return cnt;
> + return 0;
> }
>
> -static const struct file_operations i915_min_freq_fops = {
> - .owner = THIS_MODULE,
> - .open = simple_open,
> - .read = i915_min_freq_read,
> - .write = i915_min_freq_write,
> - .llseek = default_llseek,
> -};
> +DEFINE_SIMPLE_ATTRIBUTE(i915_min_freq_fops,
> + i915_min_freq_get, i915_min_freq_set,
> + "min freq: %llu\n");
>
> -static ssize_t
> -i915_cache_sharing_read(struct file *filp,
> - char __user *ubuf,
> - size_t max,
> - loff_t *ppos)
> +static int
> +i915_cache_sharing_get(void *data, u64 *val)
> {
> - struct drm_device *dev = filp->private_data;
> + struct drm_device *dev = data;
> drm_i915_private_t *dev_priv = dev->dev_private;
> - char buf[80];
> u32 snpcr;
> - int len, ret;
> + int ret;
>
> if (!(IS_GEN6(dev) || IS_GEN7(dev)))
> return -ENODEV;
> @@ -2068,46 +1895,25 @@ i915_cache_sharing_read(struct file *filp,
> snpcr = I915_READ(GEN6_MBCUNIT_SNPCR);
> mutex_unlock(&dev_priv->dev->struct_mutex);
>
> - len = snprintf(buf, sizeof(buf),
> - "%d\n", (snpcr & GEN6_MBC_SNPCR_MASK) >>
> - GEN6_MBC_SNPCR_SHIFT);
> -
> - if (len > sizeof(buf))
> - len = sizeof(buf);
> + *val = (snpcr & GEN6_MBC_SNPCR_MASK) >> GEN6_MBC_SNPCR_SHIFT;
>
> - return simple_read_from_buffer(ubuf, max, ppos, buf, len);
> + return 0;
> }
>
> -static ssize_t
> -i915_cache_sharing_write(struct file *filp,
> - const char __user *ubuf,
> - size_t cnt,
> - loff_t *ppos)
> +static int
> +i915_cache_sharing_set(void *data, u64 val)
> {
> - struct drm_device *dev = filp->private_data;
> + struct drm_device *dev = data;
> struct drm_i915_private *dev_priv = dev->dev_private;
> - char buf[20];
> u32 snpcr;
> - int val = 1;
>
> if (!(IS_GEN6(dev) || IS_GEN7(dev)))
> return -ENODEV;
>
> - if (cnt > 0) {
> - if (cnt > sizeof(buf) - 1)
> - return -EINVAL;
> -
> - if (copy_from_user(buf, ubuf, cnt))
> - return -EFAULT;
> - buf[cnt] = 0;
> -
> - val = simple_strtoul(buf, NULL, 0);
> - }
> -
> - if (val < 0 || val > 3)
> + if (val > 3)
> return -EINVAL;
>
> - DRM_DEBUG_DRIVER("Manually setting uncore sharing to %d\n", val);
> + DRM_DEBUG_DRIVER("Manually setting uncore sharing to %llu\n", val);
>
> /* Update the cache sharing policy here as well */
> snpcr = I915_READ(GEN6_MBCUNIT_SNPCR);
> @@ -2115,16 +1921,12 @@ i915_cache_sharing_write(struct file *filp,
> snpcr |= (val << GEN6_MBC_SNPCR_SHIFT);
> I915_WRITE(GEN6_MBCUNIT_SNPCR, snpcr);
>
> - return cnt;
> + return 0;
> }
>
> -static const struct file_operations i915_cache_sharing_fops = {
> - .owner = THIS_MODULE,
> - .open = simple_open,
> - .read = i915_cache_sharing_read,
> - .write = i915_cache_sharing_write,
> - .llseek = default_llseek,
> -};
> +DEFINE_SIMPLE_ATTRIBUTE(i915_cache_sharing_fops,
> + i915_cache_sharing_get, i915_cache_sharing_set,
> + "%llu\n");
>
> /* As the drm_debugfs_init() routines are called before dev->dev_private is
> * allocated we need to hook into the minor for release. */
> --
> 1.7.9.5
>
>
> --
> Kees Cook
> Chrome OS Security
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
On Mon, Mar 11, 2013 at 4:03 PM, Daniel Vetter <[email protected]> wrote:
> On Sun, Mar 10, 2013 at 02:10:06PM -0700, Kees Cook wrote:
>> This replaces the manual read/write routines in debugfs with the common
>> simple attribute helpers. Doing this gets rid of repeated copy/pasting
>> of copy_from_user and value formatting code.
>>
>> Signed-off-by: Kees Cook <[email protected]>
>> Cc: Daniel Vetter <[email protected]>
>
> Queued for -next, thanks for the patch. When sending drm/i915 patches,
> please also cc [email protected] (it's open for
> non-subscribers non without nagging "you're mail is in the moderation
> queue" replies).
Ah-ha, sure.
Should MAINTAINERS be updated to reflect this? It got skipped as a
destination due to the "subscribers-only" suffix.
-Kees
--
Kees Cook
Chrome OS Security