[PATCH] drm/i915/pmu: Hide the (unsigned long)ptr cast

[PATCH] drm/i915/pmu: Hide the (unsigned long)ptr cast

[Chris Wilson]

We pretend the PMU config id is a pointer value when encoding it into
the device parameters for presentation via sysfs. This requires casting
of an unsigned long into and out of the pointer member, which annoys
smatch:

drivers/gpu/drm/i915/i915_pmu.c:684 i915_pmu_event_show() warn: argument 3 to %lx specifier is cast from pointer

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/i915_pmu.c   | 2 +-
 drivers/gpu/drm/i915/i915_utils.h | 5 +++++
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_pmu.c b/drivers/gpu/drm/i915/i915_pmu.c
index 6a42e7f7967d..a8816d25a77b 100644
--- a/drivers/gpu/drm/i915/i915_pmu.c
+++ b/drivers/gpu/drm/i915/i915_pmu.c
@@ -681,7 +681,7 @@ static ssize_t i915_pmu_event_show(struct device *dev,
 	struct dev_ext_attribute *eattr;
 
 	eattr = container_of(attr, struct dev_ext_attribute, attr);
-	return sprintf(buf, "config=0x%lx\n", (unsigned long)eattr->var);
+	return sprintf(buf, "config=0x%lx\n", ptr_to_ulong(eattr->var));
 }
 
 #define I915_EVENT_ATTR(_name, _config) \
diff --git a/drivers/gpu/drm/i915/i915_utils.h b/drivers/gpu/drm/i915/i915_utils.h
index 8d07764887ec..2595fbf6451f 100644
--- a/drivers/gpu/drm/i915/i915_utils.h
+++ b/drivers/gpu/drm/i915/i915_utils.h
@@ -107,6 +107,11 @@ static inline u64 ptr_to_u64(const void *ptr)
 	return (uintptr_t)ptr;
 }
 
+static inline unsigned long ptr_to_ulong(const void *ptr)
+{
+	return ptr_to_u64(ptr);
+}
+
 #define u64_to_ptr(T, x) ({						\
 	typecheck(u64, x);						\
 	(T *)(uintptr_t)(x);						\
-- 
2.15.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.BAT: success for drm/i915/pmu: Hide the (unsigned long)ptr cast

[Patchwork]

== Series Details ==

Series: drm/i915/pmu: Hide the (unsigned long)ptr cast
URL   : https://patchwork.freedesktop.org/series/34318/
State : success

== Summary ==

Series 34318v1 drm/i915/pmu: Hide the (unsigned long)ptr cast
https://patchwork.freedesktop.org/api/1.0/series/34318/revisions/1/mbox/

Test kms_pipe_crc_basic:
        Subgroup suspend-read-crc-pipe-c:
                incomplete -> PASS       (fi-cfl-s2)

fi-bdw-5557u     total:289  pass:268  dwarn:0   dfail:0   fail:0   skip:21  time:449s
fi-bdw-gvtdvm    total:289  pass:265  dwarn:0   dfail:0   fail:0   skip:24  time:456s
fi-blb-e6850     total:289  pass:223  dwarn:1   dfail:0   fail:0   skip:65  time:387s
fi-bsw-n3050     total:289  pass:243  dwarn:0   dfail:0   fail:0   skip:46  time:541s
fi-bwr-2160      total:289  pass:183  dwarn:0   dfail:0   fail:0   skip:106 time:279s
fi-bxt-dsi       total:289  pass:259  dwarn:0   dfail:0   fail:0   skip:30  time:513s
fi-bxt-j4205     total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:514s
fi-byt-j1900     total:289  pass:254  dwarn:0   dfail:0   fail:0   skip:35  time:506s
fi-byt-n2820     total:289  pass:250  dwarn:0   dfail:0   fail:0   skip:39  time:493s
fi-cfl-s2        total:289  pass:263  dwarn:0   dfail:0   fail:0   skip:26  time:608s
fi-elk-e7500     total:289  pass:229  dwarn:0   dfail:0   fail:0   skip:60  time:439s
fi-gdg-551       total:289  pass:178  dwarn:1   dfail:0   fail:1   skip:109 time:274s
fi-glk-1         total:289  pass:261  dwarn:0   dfail:0   fail:0   skip:28  time:540s
fi-hsw-4770      total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:439s
fi-hsw-4770r     total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:441s
fi-ilk-650       total:289  pass:228  dwarn:0   dfail:0   fail:0   skip:61  time:425s
fi-ivb-3520m     total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:479s
fi-ivb-3770      total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:462s
fi-pnv-d510      total:289  pass:222  dwarn:1   dfail:0   fail:0   skip:66  time:576s
fi-skl-6260u     total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:462s
fi-skl-6600u     total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:549s
fi-skl-6700hq    total:289  pass:263  dwarn:0   dfail:0   fail:0   skip:26  time:568s
fi-skl-6700k     total:289  pass:265  dwarn:0   dfail:0   fail:0   skip:24  time:528s
fi-skl-6770hq    total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:502s
fi-skl-gvtdvm    total:289  pass:266  dwarn:0   dfail:0   fail:0   skip:23  time:463s
fi-snb-2520m     total:289  pass:250  dwarn:0   dfail:0   fail:0   skip:39  time:558s
fi-snb-2600      total:289  pass:249  dwarn:0   dfail:0   fail:0   skip:40  time:428s
Blacklisted hosts:
fi-cnl-y         total:231  pass:206  dwarn:0   dfail:0   fail:0   skip:24 
fi-glk-dsi       total:289  pass:259  dwarn:0   dfail:0   fail:0   skip:30  time:506s
fi-kbl-7500u     total:289  pass:264  dwarn:1   dfail:0   fail:0   skip:24  time:486s
fi-kbl-7560u     total:289  pass:270  dwarn:0   dfail:0   fail:0   skip:19  time:535s
fi-kbl-7567u     total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:472s
fi-kbl-r         total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:537s

b407e5f38397c0c22b5056a1664753287993b152 drm-tip: 2017y-11m-23d-16h-14m-59s UTC integration manifest
c58003d888da drm/i915/pmu: Hide the (unsigned long)ptr cast

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_7265/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915/pmu: Hide the (unsigned long)ptr cast

[Michal Wajdeczko]

On Thu, 23 Nov 2017 18:42:13 +0100, Chris Wilson  
<chris@chris-wilson.co.uk> wrote:

> We pretend the PMU config id is a pointer value when encoding it into
> the device parameters for presentation via sysfs. This requires casting
> of an unsigned long into and out of the pointer member, which annoys
> smatch:
>
> drivers/gpu/drm/i915/i915_pmu.c:684 i915_pmu_event_show() warn: argument  
> 3 to %lx specifier is cast from pointer
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_pmu.c   | 2 +-
>  drivers/gpu/drm/i915/i915_utils.h | 5 +++++
>  2 files changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_pmu.c  
> b/drivers/gpu/drm/i915/i915_pmu.c
> index 6a42e7f7967d..a8816d25a77b 100644
> --- a/drivers/gpu/drm/i915/i915_pmu.c
> +++ b/drivers/gpu/drm/i915/i915_pmu.c
> @@ -681,7 +681,7 @@ static ssize_t i915_pmu_event_show(struct device  
> *dev,
>  	struct dev_ext_attribute *eattr;
> 	eattr = container_of(attr, struct dev_ext_attribute, attr);
> -	return sprintf(buf, "config=0x%lx\n", (unsigned long)eattr->var);
> +	return sprintf(buf, "config=0x%lx\n", ptr_to_ulong(eattr->var));

Hmm, can't we just use %p here ?

	return sprintf(buf, "config=%p\n", eattr->var);

>  }
> #define I915_EVENT_ATTR(_name, _config) \
> diff --git a/drivers/gpu/drm/i915/i915_utils.h  
> b/drivers/gpu/drm/i915/i915_utils.h
> index 8d07764887ec..2595fbf6451f 100644
> --- a/drivers/gpu/drm/i915/i915_utils.h
> +++ b/drivers/gpu/drm/i915/i915_utils.h
> @@ -107,6 +107,11 @@ static inline u64 ptr_to_u64(const void *ptr)
>  	return (uintptr_t)ptr;
>  }
> +static inline unsigned long ptr_to_ulong(const void *ptr)
> +{
> +	return ptr_to_u64(ptr);
> +}
> +
>  #define u64_to_ptr(T, x) ({						\
>  	typecheck(u64, x);						\
>  	(T *)(uintptr_t)(x);						\
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915/pmu: Hide the (unsigned long)ptr cast

[Chris Wilson]

Quoting Michal Wajdeczko (2017-11-23 18:20:52)
> On Thu, 23 Nov 2017 18:42:13 +0100, Chris Wilson  
> <chris@chris-wilson.co.uk> wrote:
> 
> > We pretend the PMU config id is a pointer value when encoding it into
> > the device parameters for presentation via sysfs. This requires casting
> > of an unsigned long into and out of the pointer member, which annoys
> > smatch:
> >
> > drivers/gpu/drm/i915/i915_pmu.c:684 i915_pmu_event_show() warn: argument  
> > 3 to %lx specifier is cast from pointer
> >
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_pmu.c   | 2 +-
> >  drivers/gpu/drm/i915/i915_utils.h | 5 +++++
> >  2 files changed, 6 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_pmu.c  
> > b/drivers/gpu/drm/i915/i915_pmu.c
> > index 6a42e7f7967d..a8816d25a77b 100644
> > --- a/drivers/gpu/drm/i915/i915_pmu.c
> > +++ b/drivers/gpu/drm/i915/i915_pmu.c
> > @@ -681,7 +681,7 @@ static ssize_t i915_pmu_event_show(struct device  
> > *dev,
> >       struct dev_ext_attribute *eattr;
> >       eattr = container_of(attr, struct dev_ext_attribute, attr);
> > -     return sprintf(buf, "config=0x%lx\n", (unsigned long)eattr->var);
> > +     return sprintf(buf, "config=0x%lx\n", ptr_to_ulong(eattr->var));
> 
> Hmm, can't we just use %p here ?

We could but "%p" raises eyebrows as to leaking kernel addresses. It
would be nicer if there was a integer attribute we could use instead. If
push came to shove, we could just define our own, I believe.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v2] drm/i915/pmu: Hide the (unsigned long)ptr cast

[Chris Wilson]

We pretend the PMU config id is a pointer value when encoding it into
the device parameters for presentation via sysfs. This requires casting
of an unsigned long into and out of the pointer member, which annoys
smatch:

drivers/gpu/drm/i915/i915_pmu.c:684 i915_pmu_event_show() warn: argument 3 to %lx specifier is cast from pointer

Instead of abusing a generic dev_ext_attribute, define our own typesafe
attributes.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
---
 drivers/gpu/drm/i915/i915_pmu.c | 30 ++++++++++++++++++++----------
 1 file changed, 20 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_pmu.c b/drivers/gpu/drm/i915/i915_pmu.c
index 6a42e7f7967d..5170a46893d7 100644
--- a/drivers/gpu/drm/i915/i915_pmu.c
+++ b/drivers/gpu/drm/i915/i915_pmu.c
@@ -650,19 +650,24 @@ static int i915_pmu_event_event_idx(struct perf_event *event)
 	return 0;
 }
 
+struct i915_str_attribute {
+	struct device_attribute attr;
+	const char *str;
+};
+
 static ssize_t i915_pmu_format_show(struct device *dev,
 				    struct device_attribute *attr, char *buf)
 {
-	struct dev_ext_attribute *eattr;
+	struct i915_str_attribute *eattr;
 
-	eattr = container_of(attr, struct dev_ext_attribute, attr);
-	return sprintf(buf, "%s\n", (char *)eattr->var);
+	eattr = container_of(attr, struct i915_str_attribute, attr);
+	return sprintf(buf, "%s\n", eattr->str);
 }
 
 #define I915_PMU_FORMAT_ATTR(_name, _config) \
-	(&((struct dev_ext_attribute[]) { \
+	(&((struct i915_str_attribute[]) { \
 		{ .attr = __ATTR(_name, 0444, i915_pmu_format_show, NULL), \
-		  .var = (void *)_config, } \
+		  .str = _config, } \
 	})[0].attr.attr)
 
 static struct attribute *i915_pmu_format_attrs[] = {
@@ -675,19 +680,24 @@ static const struct attribute_group i915_pmu_format_attr_group = {
 	.attrs = i915_pmu_format_attrs,
 };
 
+struct i915_ext_attribute {
+	struct device_attribute attr;
+	unsigned long val;
+};
+
 static ssize_t i915_pmu_event_show(struct device *dev,
 				   struct device_attribute *attr, char *buf)
 {
-	struct dev_ext_attribute *eattr;
+	struct i915_ext_attribute *eattr;
 
-	eattr = container_of(attr, struct dev_ext_attribute, attr);
-	return sprintf(buf, "config=0x%lx\n", (unsigned long)eattr->var);
+	eattr = container_of(attr, struct i915_ext_attribute, attr);
+	return sprintf(buf, "config=0x%lx\n", eattr->val);
 }
 
 #define I915_EVENT_ATTR(_name, _config) \
-	(&((struct dev_ext_attribute[]) { \
+	(&((struct i915_ext_attribute[]) { \
 		{ .attr = __ATTR(_name, 0444, i915_pmu_event_show, NULL), \
-		  .var = (void *)_config, } \
+		  .val = _config, } \
 	})[0].attr.attr)
 
 #define I915_EVENT_STR(_name, _str) \
-- 
2.15.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.IGT: warning for drm/i915/pmu: Hide the (unsigned long)ptr cast

[Patchwork]

== Series Details ==

Series: drm/i915/pmu: Hide the (unsigned long)ptr cast
URL   : https://patchwork.freedesktop.org/series/34318/
State : warning

== Summary ==

Test kms_frontbuffer_tracking:
        Subgroup fbc-1p-offscren-pri-shrfb-draw-blt:
                pass       -> FAIL       (shard-snb) fdo#101623
Test kms_setmode:
        Subgroup basic:
                fail       -> PASS       (shard-hsw) fdo#99912
Test kms_chv_cursor_fail:
        Subgroup pipe-b-128x128-top-edge:
                pass       -> SKIP       (shard-hsw)
Test kms_force_connector_basic:
        Subgroup prune-stale-modes:
                skip       -> PASS       (shard-hsw)
Test drv_module_reload:
        Subgroup basic-no-display:
                pass       -> DMESG-WARN (shard-snb) fdo#102707 +1

fdo#101623 https://bugs.freedesktop.org/show_bug.cgi?id=101623
fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912
fdo#102707 https://bugs.freedesktop.org/show_bug.cgi?id=102707

shard-hsw        total:2667 pass:1534 dwarn:2   dfail:0   fail:10  skip:1121 time:9516s
shard-snb        total:2667 pass:1311 dwarn:2   dfail:0   fail:13  skip:1341 time:8101s

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_7265/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.BAT: success for drm/i915/pmu: Hide the (unsigned long)ptr cast (rev2)

[Patchwork]

== Series Details ==

Series: drm/i915/pmu: Hide the (unsigned long)ptr cast (rev2)
URL   : https://patchwork.freedesktop.org/series/34318/
State : success

== Summary ==

Series 34318v2 drm/i915/pmu: Hide the (unsigned long)ptr cast
https://patchwork.freedesktop.org/api/1.0/series/34318/revisions/2/mbox/

Test kms_pipe_crc_basic:
        Subgroup suspend-read-crc-pipe-c:
                incomplete -> PASS       (fi-cfl-s2)

fi-bdw-5557u     total:289  pass:268  dwarn:0   dfail:0   fail:0   skip:21  time:444s
fi-bdw-gvtdvm    total:289  pass:265  dwarn:0   dfail:0   fail:0   skip:24  time:459s
fi-blb-e6850     total:289  pass:223  dwarn:1   dfail:0   fail:0   skip:65  time:382s
fi-bsw-n3050     total:289  pass:243  dwarn:0   dfail:0   fail:0   skip:46  time:554s
fi-bwr-2160      total:289  pass:183  dwarn:0   dfail:0   fail:0   skip:106 time:276s
fi-bxt-dsi       total:289  pass:259  dwarn:0   dfail:0   fail:0   skip:30  time:502s
fi-bxt-j4205     total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:512s
fi-byt-j1900     total:289  pass:254  dwarn:0   dfail:0   fail:0   skip:35  time:501s
fi-byt-n2820     total:289  pass:250  dwarn:0   dfail:0   fail:0   skip:39  time:493s
fi-cfl-s2        total:289  pass:263  dwarn:0   dfail:0   fail:0   skip:26  time:602s
fi-elk-e7500     total:289  pass:229  dwarn:0   dfail:0   fail:0   skip:60  time:432s
fi-gdg-551       total:289  pass:178  dwarn:1   dfail:0   fail:1   skip:109 time:267s
fi-glk-1         total:289  pass:261  dwarn:0   dfail:0   fail:0   skip:28  time:543s
fi-hsw-4770      total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:432s
fi-hsw-4770r     total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:441s
fi-ilk-650       total:289  pass:228  dwarn:0   dfail:0   fail:0   skip:61  time:441s
fi-ivb-3520m     total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:487s
fi-ivb-3770      total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:463s
fi-pnv-d510      total:289  pass:222  dwarn:1   dfail:0   fail:0   skip:66  time:580s
fi-skl-6260u     total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:458s
fi-skl-6600u     total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:550s
fi-skl-6700hq    total:289  pass:263  dwarn:0   dfail:0   fail:0   skip:26  time:570s
fi-skl-6700k     total:289  pass:265  dwarn:0   dfail:0   fail:0   skip:24  time:518s
fi-skl-6770hq    total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:496s
fi-skl-gvtdvm    total:289  pass:266  dwarn:0   dfail:0   fail:0   skip:23  time:467s
fi-snb-2520m     total:289  pass:250  dwarn:0   dfail:0   fail:0   skip:39  time:567s
fi-snb-2600      total:289  pass:249  dwarn:0   dfail:0   fail:0   skip:40  time:424s
Blacklisted hosts:
fi-glk-dsi       total:289  pass:259  dwarn:0   dfail:0   fail:0   skip:30  time:504s
fi-kbl-7500u     total:289  pass:264  dwarn:1   dfail:0   fail:0   skip:24  time:488s
fi-kbl-7560u     total:289  pass:270  dwarn:0   dfail:0   fail:0   skip:19  time:532s
fi-kbl-7567u     total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:479s
fi-kbl-r         total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:536s

b407e5f38397c0c22b5056a1664753287993b152 drm-tip: 2017y-11m-23d-16h-14m-59s UTC integration manifest
8fc005f6ba8f drm/i915/pmu: Hide the (unsigned long)ptr cast

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_7270/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.IGT: success for drm/i915/pmu: Hide the (unsigned long)ptr cast (rev2)

[Patchwork]

== Series Details ==

Series: drm/i915/pmu: Hide the (unsigned long)ptr cast (rev2)
URL   : https://patchwork.freedesktop.org/series/34318/
State : success

== Summary ==

Test kms_frontbuffer_tracking:
        Subgroup fbc-1p-offscren-pri-shrfb-draw-blt:
                pass       -> FAIL       (shard-snb) fdo#101623
Test perf:
        Subgroup polling:
                fail       -> PASS       (shard-hsw) fdo#102252
Test drv_selftest:
        Subgroup mock_sanitycheck:
                pass       -> DMESG-WARN (shard-hsw) fdo#103719
Test drv_module_reload:
        Subgroup basic-reload-inject:
                pass       -> DMESG-WARN (shard-snb) fdo#102707 +1

fdo#101623 https://bugs.freedesktop.org/show_bug.cgi?id=101623
fdo#102252 https://bugs.freedesktop.org/show_bug.cgi?id=102252
fdo#103719 https://bugs.freedesktop.org/show_bug.cgi?id=103719
fdo#102707 https://bugs.freedesktop.org/show_bug.cgi?id=102707

shard-hsw        total:2667 pass:1534 dwarn:2   dfail:0   fail:10  skip:1121 time:9530s
shard-snb        total:2667 pass:1310 dwarn:3   dfail:0   fail:13  skip:1341 time:8115s

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_7270/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915/pmu: Hide the (unsigned long)ptr cast

[Tvrtko Ursulin]

On 23/11/2017 17:42, Chris Wilson wrote:
> We pretend the PMU config id is a pointer value when encoding it into
> the device parameters for presentation via sysfs. This requires casting
> of an unsigned long into and out of the pointer member, which annoys
> smatch:
> 
> drivers/gpu/drm/i915/i915_pmu.c:684 i915_pmu_event_show() warn: argument 3 to %lx specifier is cast from pointer

Why is it even complaining about this? Maybe it is early but I don't see 
a problem.

Regards,

Tvrtko

> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
>   drivers/gpu/drm/i915/i915_pmu.c   | 2 +-
>   drivers/gpu/drm/i915/i915_utils.h | 5 +++++
>   2 files changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_pmu.c b/drivers/gpu/drm/i915/i915_pmu.c
> index 6a42e7f7967d..a8816d25a77b 100644
> --- a/drivers/gpu/drm/i915/i915_pmu.c
> +++ b/drivers/gpu/drm/i915/i915_pmu.c
> @@ -681,7 +681,7 @@ static ssize_t i915_pmu_event_show(struct device *dev,
>   	struct dev_ext_attribute *eattr;
>   
>   	eattr = container_of(attr, struct dev_ext_attribute, attr);
> -	return sprintf(buf, "config=0x%lx\n", (unsigned long)eattr->var);
> +	return sprintf(buf, "config=0x%lx\n", ptr_to_ulong(eattr->var));
>   }
>   
>   #define I915_EVENT_ATTR(_name, _config) \
> diff --git a/drivers/gpu/drm/i915/i915_utils.h b/drivers/gpu/drm/i915/i915_utils.h
> index 8d07764887ec..2595fbf6451f 100644
> --- a/drivers/gpu/drm/i915/i915_utils.h
> +++ b/drivers/gpu/drm/i915/i915_utils.h
> @@ -107,6 +107,11 @@ static inline u64 ptr_to_u64(const void *ptr)
>   	return (uintptr_t)ptr;
>   }
>   
> +static inline unsigned long ptr_to_ulong(const void *ptr)
> +{
> +	return ptr_to_u64(ptr);
> +}
> +
>   #define u64_to_ptr(T, x) ({						\
>   	typecheck(u64, x);						\
>   	(T *)(uintptr_t)(x);						\
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2] drm/i915/pmu: Hide the (unsigned long)ptr cast

[Tvrtko Ursulin]

On 23/11/2017 21:17, Chris Wilson wrote:
> We pretend the PMU config id is a pointer value when encoding it into
> the device parameters for presentation via sysfs. This requires casting
> of an unsigned long into and out of the pointer member, which annoys
> smatch:
> 
> drivers/gpu/drm/i915/i915_pmu.c:684 i915_pmu_event_show() warn: argument 3 to %lx specifier is cast from pointer
> 
> Instead of abusing a generic dev_ext_attribute, define our own typesafe
> attributes.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> ---
>   drivers/gpu/drm/i915/i915_pmu.c | 30 ++++++++++++++++++++----------
>   1 file changed, 20 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_pmu.c b/drivers/gpu/drm/i915/i915_pmu.c
> index 6a42e7f7967d..5170a46893d7 100644
> --- a/drivers/gpu/drm/i915/i915_pmu.c
> +++ b/drivers/gpu/drm/i915/i915_pmu.c
> @@ -650,19 +650,24 @@ static int i915_pmu_event_event_idx(struct perf_event *event)
>   	return 0;
>   }
>   
> +struct i915_str_attribute {
> +	struct device_attribute attr;
> +	const char *str;
> +};
> +
>   static ssize_t i915_pmu_format_show(struct device *dev,
>   				    struct device_attribute *attr, char *buf)
>   {
> -	struct dev_ext_attribute *eattr;
> +	struct i915_str_attribute *eattr;
>   
> -	eattr = container_of(attr, struct dev_ext_attribute, attr);
> -	return sprintf(buf, "%s\n", (char *)eattr->var);
> +	eattr = container_of(attr, struct i915_str_attribute, attr);
> +	return sprintf(buf, "%s\n", eattr->str);
>   }
>   
>   #define I915_PMU_FORMAT_ATTR(_name, _config) \
> -	(&((struct dev_ext_attribute[]) { \
> +	(&((struct i915_str_attribute[]) { \
>   		{ .attr = __ATTR(_name, 0444, i915_pmu_format_show, NULL), \
> -		  .var = (void *)_config, } \
> +		  .str = _config, } \
>   	})[0].attr.attr)
>   
>   static struct attribute *i915_pmu_format_attrs[] = {
> @@ -675,19 +680,24 @@ static const struct attribute_group i915_pmu_format_attr_group = {
>   	.attrs = i915_pmu_format_attrs,
>   };
>   
> +struct i915_ext_attribute {
> +	struct device_attribute attr;
> +	unsigned long val;
> +};
> +
>   static ssize_t i915_pmu_event_show(struct device *dev,
>   				   struct device_attribute *attr, char *buf)
>   {
> -	struct dev_ext_attribute *eattr;
> +	struct i915_ext_attribute *eattr;
>   
> -	eattr = container_of(attr, struct dev_ext_attribute, attr);
> -	return sprintf(buf, "config=0x%lx\n", (unsigned long)eattr->var);
> +	eattr = container_of(attr, struct i915_ext_attribute, attr);
> +	return sprintf(buf, "config=0x%lx\n", eattr->val);
>   }
>   
>   #define I915_EVENT_ATTR(_name, _config) \
> -	(&((struct dev_ext_attribute[]) { \
> +	(&((struct i915_ext_attribute[]) { \
>   		{ .attr = __ATTR(_name, 0444, i915_pmu_event_show, NULL), \
> -		  .var = (void *)_config, } \
> +		  .val = _config, } \
>   	})[0].attr.attr)
>   
>   #define I915_EVENT_STR(_name, _str) \
> 

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Regards,

Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915/pmu: Hide the (unsigned long)ptr cast

[Chris Wilson]

Quoting Tvrtko Ursulin (2017-11-24 08:04:28)
> 
> On 23/11/2017 17:42, Chris Wilson wrote:
> > We pretend the PMU config id is a pointer value when encoding it into
> > the device parameters for presentation via sysfs. This requires casting
> > of an unsigned long into and out of the pointer member, which annoys
> > smatch:
> > 
> > drivers/gpu/drm/i915/i915_pmu.c:684 i915_pmu_event_show() warn: argument 3 to %lx specifier is cast from pointer
> 
> Why is it even complaining about this? Maybe it is early but I don't see 
> a problem.

I believe it is to catch %lx being used to bypass the security around
%p.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2] drm/i915/pmu: Hide the (unsigned long)ptr cast

[Chris Wilson]

Quoting Tvrtko Ursulin (2017-11-24 08:42:06)
> 
> On 23/11/2017 21:17, Chris Wilson wrote:
> > We pretend the PMU config id is a pointer value when encoding it into
> > the device parameters for presentation via sysfs. This requires casting
> > of an unsigned long into and out of the pointer member, which annoys
> > smatch:
> > 
> > drivers/gpu/drm/i915/i915_pmu.c:684 i915_pmu_event_show() warn: argument 3 to %lx specifier is cast from pointer
> > 
> > Instead of abusing a generic dev_ext_attribute, define our own typesafe
> > attributes.
> > 
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> > Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
[snip]
> Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Thanks, it's such a trivial warning that I wasn't such if it was worth
the effort. But we are close to being smatch clean, and in the past it
has caught genuine bugs that gcc/sparse miss so I think it is a valuable
tool, and so worth the effort to get and stay clean.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx