[PATCH i-g-t v4 0/2] 32/64-bit ptr wrapper

[PATCH i-g-t v4 0/2] 32/64-bit ptr wrapper

[Robert Foss]

Added and applied 32/64-bit wrapper for pointers used in ioctls.

Robert Foss (2):
  tests/perf: Fix pointer length compilation errors on 32-bit systems
  lib/ioctl_wrappers.h: Add to_user_pointer() helper

 lib/ioctl_wrappers.h | 11 +++++++++++
 tests/perf.c         | 42 +++++++++++++++++++++---------------------
 2 files changed, 32 insertions(+), 21 deletions(-)

-- 
2.11.0

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

[PATCH i-g-t v4 1/2] tests/perf: Fix pointer length compilation errors on 32-bit systems

[Robert Foss]

Fix pointer length compilations errors on 32-bit systems.

Signed-off-by: Robert Foss <robert.foss@collabora.com>
---
 tests/perf.c | 42 +++++++++++++++++++++---------------------
 1 file changed, 21 insertions(+), 21 deletions(-)

diff --git a/tests/perf.c b/tests/perf.c
index 87df9f00..c9c5c57e 100644
--- a/tests/perf.c
+++ b/tests/perf.c
@@ -432,7 +432,7 @@ test_system_wide_paranoid(void)
 			.flags = I915_PERF_FLAG_FD_CLOEXEC |
 				I915_PERF_FLAG_FD_NONBLOCK,
 			.num_properties = sizeof(properties) / 16,
-			.properties_ptr = (uint64_t)properties,
+			.properties_ptr = to_user_pointer(properties),
 		};
 
 		write_u64_file("/proc/sys/dev/i915/perf_stream_paranoid", 1);
@@ -458,7 +458,7 @@ test_system_wide_paranoid(void)
 			.flags = I915_PERF_FLAG_FD_CLOEXEC |
 				I915_PERF_FLAG_FD_NONBLOCK,
 			.num_properties = sizeof(properties) / 16,
-			.properties_ptr = (uint64_t)properties,
+			.properties_ptr = to_user_pointer(properties),
 		};
 		int stream_fd;
 
@@ -491,7 +491,7 @@ test_invalid_open_flags(void)
 	struct drm_i915_perf_open_param param = {
 		.flags = ~0, /* Undefined flag bits set! */
 		.num_properties = sizeof(properties) / 16,
-		.properties_ptr = (uint64_t)properties,
+		.properties_ptr = to_user_pointer(properties),
 	};
 
 	do_ioctl_err(drm_fd, DRM_IOCTL_I915_PERF_OPEN, &param, EINVAL);
@@ -513,7 +513,7 @@ test_invalid_oa_metric_set_id(void)
 		.flags = I915_PERF_FLAG_FD_CLOEXEC |
 			I915_PERF_FLAG_FD_NONBLOCK,
 		.num_properties = sizeof(properties) / 16,
-		.properties_ptr = (uint64_t)properties,
+		.properties_ptr = to_user_pointer(properties),
 	};
 	int stream_fd;
 
@@ -548,7 +548,7 @@ test_invalid_oa_format_id(void)
 		.flags = I915_PERF_FLAG_FD_CLOEXEC |
 			I915_PERF_FLAG_FD_NONBLOCK,
 		.num_properties = sizeof(properties) / 16,
-		.properties_ptr = (uint64_t)properties,
+		.properties_ptr = to_user_pointer(properties),
 	};
 	int stream_fd;
 
@@ -581,7 +581,7 @@ test_missing_sample_flags(void)
 	struct drm_i915_perf_open_param param = {
 		.flags = I915_PERF_FLAG_FD_CLOEXEC,
 		.num_properties = sizeof(properties) / 16,
-		.properties_ptr = (uint64_t)properties,
+		.properties_ptr = to_user_pointer(properties),
 	};
 
 	do_ioctl_err(drm_fd, DRM_IOCTL_I915_PERF_OPEN, &param, EINVAL);
@@ -735,7 +735,7 @@ open_and_read_2_oa_reports(int format_id,
 	struct drm_i915_perf_open_param param = {
 		.flags = I915_PERF_FLAG_FD_CLOEXEC,
 		.num_properties = sizeof(properties) / 16,
-		.properties_ptr = (uint64_t)properties,
+		.properties_ptr = to_user_pointer(properties),
 	};
 	int stream_fd = __perf_open(drm_fd, &param);
 
@@ -1047,7 +1047,7 @@ test_invalid_oa_exponent(void)
 	struct drm_i915_perf_open_param param = {
 		.flags = I915_PERF_FLAG_FD_CLOEXEC,
 		.num_properties = sizeof(properties) / 16,
-		.properties_ptr = (uint64_t)properties,
+		.properties_ptr = to_user_pointer(properties),
 	};
 	int stream_fd = __perf_open(drm_fd, &param);
 
@@ -1103,7 +1103,7 @@ test_low_oa_exponent_permissions(void)
 	struct drm_i915_perf_open_param param = {
 		.flags = I915_PERF_FLAG_FD_CLOEXEC,
 		.num_properties = sizeof(properties) / 16,
-		.properties_ptr = (uint64_t)properties,
+		.properties_ptr = to_user_pointer(properties),
 	};
 	uint64_t oa_period, oa_freq;
 
@@ -1168,7 +1168,7 @@ test_per_context_mode_unprivileged(void)
 	struct drm_i915_perf_open_param param = {
 		.flags = I915_PERF_FLAG_FD_CLOEXEC,
 		.num_properties = sizeof(properties) / 16,
-		.properties_ptr = (uint64_t)properties,
+		.properties_ptr = to_user_pointer(properties),
 	};
 
 	/* should be default, but just to be sure... */
@@ -1255,7 +1255,7 @@ test_blocking(void)
 	struct drm_i915_perf_open_param param = {
 		.flags = I915_PERF_FLAG_FD_CLOEXEC,
 		.num_properties = sizeof(properties) / 16,
-		.properties_ptr = (uint64_t)properties,
+		.properties_ptr = to_user_pointer(properties),
 	};
 	int stream_fd = __perf_open(drm_fd, &param);
 	uint8_t buf[1024 * 1024];
@@ -1348,7 +1348,7 @@ test_polling(void)
 		.flags = I915_PERF_FLAG_FD_CLOEXEC |
 			I915_PERF_FLAG_FD_NONBLOCK,
 		.num_properties = sizeof(properties) / 16,
-		.properties_ptr = (uint64_t)properties,
+		.properties_ptr = to_user_pointer(properties),
 	};
 	int stream_fd = __perf_open(drm_fd, &param);
 	uint8_t buf[1024 * 1024];
@@ -1465,7 +1465,7 @@ test_buffer_fill(void)
 	struct drm_i915_perf_open_param param = {
 		.flags = I915_PERF_FLAG_FD_CLOEXEC,
 		.num_properties = sizeof(properties) / 16,
-		.properties_ptr = (uint64_t)properties,
+		.properties_ptr = to_user_pointer(properties),
 	};
 	int stream_fd = __perf_open(drm_fd, &param);
 	int buf_size = 65536 * (256 + sizeof(struct drm_i915_perf_record_header));
@@ -1540,7 +1540,7 @@ test_enable_disable(void)
 		.flags = I915_PERF_FLAG_FD_CLOEXEC |
 			 I915_PERF_FLAG_DISABLED, /* Verify we start disabled */
 		.num_properties = sizeof(properties) / 16,
-		.properties_ptr = (uint64_t)properties,
+		.properties_ptr = to_user_pointer(properties),
 	};
 	int stream_fd = __perf_open(drm_fd, &param);
 	int buf_size = 65536 * (256 + sizeof(struct drm_i915_perf_record_header));
@@ -1610,7 +1610,7 @@ test_short_reads(void)
 	struct drm_i915_perf_open_param param = {
 		.flags = I915_PERF_FLAG_FD_CLOEXEC,
 		.num_properties = sizeof(properties) / 16,
-		.properties_ptr = (uint64_t)properties,
+		.properties_ptr = to_user_pointer(properties),
 	};
 	size_t record_size = 256 + sizeof(struct drm_i915_perf_record_header);
 	size_t page_size = sysconf(_SC_PAGE_SIZE);
@@ -1698,7 +1698,7 @@ test_non_sampling_read_error(void)
 	struct drm_i915_perf_open_param param = {
 		.flags = I915_PERF_FLAG_FD_CLOEXEC,
 		.num_properties = sizeof(properties) / 16,
-		.properties_ptr = (uint64_t)properties,
+		.properties_ptr = to_user_pointer(properties),
 	};
 	int stream_fd = __perf_open(drm_fd, &param);
 	uint8_t buf[1024];
@@ -1732,7 +1732,7 @@ test_disabled_read_error(void)
 		.flags = I915_PERF_FLAG_FD_CLOEXEC |
 			 I915_PERF_FLAG_DISABLED, /* XXX: open disabled */
 		.num_properties = sizeof(properties) / 16,
-		.properties_ptr = (uint64_t)properties,
+		.properties_ptr = to_user_pointer(properties),
 	};
 	int stream_fd = __perf_open(drm_fd, &param);
 	uint32_t oa_report0[64];
@@ -1794,7 +1794,7 @@ test_mi_rpc(void)
 	struct drm_i915_perf_open_param param = {
 		.flags = I915_PERF_FLAG_FD_CLOEXEC,
 		.num_properties = sizeof(properties) / 16,
-		.properties_ptr = (uint64_t)properties,
+		.properties_ptr = to_user_pointer(properties),
 	};
 	int stream_fd = __perf_open(drm_fd, &param);
 	drm_intel_bufmgr *bufmgr = drm_intel_bufmgr_gem_init(drm_fd, 4096);
@@ -1924,7 +1924,7 @@ test_per_ctx_mi_rpc(void)
 	struct drm_i915_perf_open_param param = {
 		.flags = I915_PERF_FLAG_FD_CLOEXEC,
 		.num_properties = sizeof(properties) / 16,
-		.properties_ptr = (uint64_t)properties,
+		.properties_ptr = to_user_pointer(properties),
 	};
 
 	/* should be default, but just to be sure... */
@@ -2134,7 +2134,7 @@ test_rc6_disable(void)
 	struct drm_i915_perf_open_param param = {
 		.flags = I915_PERF_FLAG_FD_CLOEXEC,
 		.num_properties = sizeof(properties) / 16,
-		.properties_ptr = (uint64_t)properties,
+		.properties_ptr = to_user_pointer(properties),
 	};
 	int stream_fd = __perf_open(drm_fd, &param);
 	uint64_t n_events_start = read_debugfs_u64_record("i915_drpc_info",
@@ -2208,7 +2208,7 @@ test_i915_ref_count(void)
 	struct drm_i915_perf_open_param param = {
 		.flags = I915_PERF_FLAG_FD_CLOEXEC,
 		.num_properties = sizeof(properties) / 16,
-		.properties_ptr = (uint64_t)properties,
+		.properties_ptr = to_user_pointer(properties),
 	};
 	unsigned baseline, ref_count0, ref_count1;
 	int stream_fd;
-- 
2.11.0

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

[PATCH i-g-t v4 2/2] lib/ioctl_wrappers.h: Add to_user_pointer() helper

[Robert Foss]

Add to_user_pointer() helper function which helps cast
pointers properly when being used with ioctls.

Signed-off-by: Robert Foss <robert.foss@collabora.com>
---
 lib/ioctl_wrappers.h | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/lib/ioctl_wrappers.h b/lib/ioctl_wrappers.h
index 26270975..2ac57f47 100644
--- a/lib/ioctl_wrappers.h
+++ b/lib/ioctl_wrappers.h
@@ -239,4 +239,15 @@ int __kms_addfb(int fd, uint32_t handle, uint32_t width, uint32_t height,
 		uint32_t stride, uint32_t pixel_format, uint64_t modifier,
 		uint32_t flags, uint32_t *buf_id);
 
+/**
+ * to_user_pointer:
+ *
+ * Makes sure that pointer on 32 and 64-bit systems
+ * are casted properly for being sent through an ioctl.
+ */
+inline uint64_t to_user_pointer(void *ptr)
+{
+	return (uint64_t)(uintptr_t) ptr;
+}
+
 #endif /* IOCTL_WRAPPERS_H */
-- 
2.11.0

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

Re: [PATCH i-g-t v4 0/2] 32/64-bit ptr wrapper

[Chris Wilson]

On Mon, Jan 02, 2017 at 05:25:53AM -0500, Robert Foss wrote:
> Added and applied 32/64-bit wrapper for pointers used in ioctls.
> 
> Robert Foss (2):
>   tests/perf: Fix pointer length compilation errors on 32-bit systems
>   lib/ioctl_wrappers.h: Add to_user_pointer() helper

How come these ended up in igt with my sign-off? The patches are
backwards and do not complie cleanly (missed static on the inline and an
unneed cast since the u64 return provides the implicit cast).
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH i-g-t v4 0/2] 32/64-bit ptr wrapper

[Robert Foss]

On 2017-01-02 05:42 AM, Chris Wilson wrote:
> On Mon, Jan 02, 2017 at 05:25:53AM -0500, Robert Foss wrote:
>> Added and applied 32/64-bit wrapper for pointers used in ioctls.
>>
>> Robert Foss (2):
>>   tests/perf: Fix pointer length compilation errors on 32-bit systems
>>   lib/ioctl_wrappers.h: Add to_user_pointer() helper
>
> How come these ended up in igt with my sign-off? The patches are
> backwards and do not complie cleanly (missed static on the inline and an
> unneed cast since the u64 return provides the implicit cast).
> -Chris

I interpreted your lgtm in v3 as a sign-off, obviously you did not 
intend that. Sorry about the nomenclature confusion.

Given that I slightly messed the push up, how would you like me to 
proceed? Fix the missed static and unneeded cast on a separate patch?


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

Re: [PATCH i-g-t v4 0/2] 32/64-bit ptr wrapper

[Chris Wilson]

On Mon, Jan 02, 2017 at 06:43:00AM -0500, Robert Foss wrote:
> 
> 
> On 2017-01-02 05:42 AM, Chris Wilson wrote:
> >On Mon, Jan 02, 2017 at 05:25:53AM -0500, Robert Foss wrote:
> >>Added and applied 32/64-bit wrapper for pointers used in ioctls.
> >>
> >>Robert Foss (2):
> >>  tests/perf: Fix pointer length compilation errors on 32-bit systems
> >>  lib/ioctl_wrappers.h: Add to_user_pointer() helper
> >
> >How come these ended up in igt with my sign-off? The patches are
> >backwards and do not complie cleanly (missed static on the inline and an
> >unneed cast since the u64 return provides the implicit cast).
> >-Chris
> 
> I interpreted your lgtm in v3 as a sign-off, obviously you did not
> intend that. Sorry about the nomenclature confusion.
> 
> Given that I slightly messed the push up, how would you like me to
> proceed? Fix the missed static and unneeded cast on a separate
> patch?

Already done.
There are a few more potential users of to_user_pointer() I haven't
checked over yet...
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx