* [PATCH 0/3] RFC: Scratch page checking
@ 2017-12-05 7:48 kevin.rogovin
2017-12-05 7:48 ` [PATCH 1/3] drm-uapi: define interface to kernel for scratch page read kevin.rogovin
` (2 more replies)
0 siblings, 3 replies; 11+ messages in thread
From: kevin.rogovin @ 2017-12-05 7:48 UTC (permalink / raw)
To: intel-gfx; +Cc: Kevin Rogovin
From: Kevin Rogovin <kevin.rogovin@intel.com>
This patch series proposes a new kernel interface for user
space to read and write the values of the scratch page for
a PPGTT. The user space is expected to guarantee (via its
own locking mechanism) that nothing shall read or write to
the scratch page for the duration of the ioctl's to read
or write the scratch page values. The purpose for user space
to read and write the scratch page values is to help see if
an out-of-bound write was done to the scratch page by the
GPU; from the point of view of GL, this can be used to help
detect if an application performs an out-of-bounds write to
an SSBO.
Patch 1 defines the kernel interface
Patches 2-3 implement the debug option in i965 to check for
inadvertent scratch page writes by the GPU.
Kevin Rogovin (3):
drm-uapi: define interface to kernel for scratch page read
i965: define stuff for scratch page checking in intel_screen
i965: check scratch page in a locked fashion on each ioctl
include/drm-uapi/i915_drm.h | 31 +++++++++++++++++++++++++++
src/intel/common/gen_debug.c | 1 +
src/intel/common/gen_debug.h | 1 +
src/mesa/drivers/dri/i965/intel_batchbuffer.c | 27 ++++++++++++++++++++++-
src/mesa/drivers/dri/i965/intel_screen.c | 26 ++++++++++++++++++++++
src/mesa/drivers/dri/i965/intel_screen.h | 12 +++++++++++
6 files changed, 97 insertions(+), 1 deletion(-)
--
2.15.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 11+ messages in thread* [PATCH 1/3] drm-uapi: define interface to kernel for scratch page read 2017-12-05 7:48 [PATCH 0/3] RFC: Scratch page checking kevin.rogovin @ 2017-12-05 7:48 ` kevin.rogovin 2017-12-05 7:57 ` Rogovin, Kevin 2017-12-05 7:48 ` [PATCH 2/3] i965: define stuff for scratch page checking in intel_screen kevin.rogovin 2017-12-05 7:48 ` [PATCH 3/3] i965: check scratch page in a locked fashion on each ioctl kevin.rogovin 2 siblings, 1 reply; 11+ messages in thread From: kevin.rogovin @ 2017-12-05 7:48 UTC (permalink / raw) To: intel-gfx; +Cc: Kevin Rogovin From: Kevin Rogovin <kevin.rogovin@intel.com> --- include/drm-uapi/i915_drm.h | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/include/drm-uapi/i915_drm.h b/include/drm-uapi/i915_drm.h index 890df227ae..3a9c3a2d0c 100644 --- a/include/drm-uapi/i915_drm.h +++ b/include/drm-uapi/i915_drm.h @@ -262,6 +262,8 @@ typedef struct _drm_i915_sarea { #define DRM_I915_PERF_OPEN 0x36 #define DRM_I915_PERF_ADD_CONFIG 0x37 #define DRM_I915_PERF_REMOVE_CONFIG 0x38 +#define DRM_I915_READ_SCRATCH_PAGE 0x39 +#define DRM_I915_WRITE_SCRATCH_PAGE 0x40 #define DRM_IOCTL_I915_INIT DRM_IOW( DRM_COMMAND_BASE + DRM_I915_INIT, drm_i915_init_t) #define DRM_IOCTL_I915_FLUSH DRM_IO ( DRM_COMMAND_BASE + DRM_I915_FLUSH) @@ -319,6 +321,8 @@ typedef struct _drm_i915_sarea { #define DRM_IOCTL_I915_PERF_OPEN DRM_IOW(DRM_COMMAND_BASE + DRM_I915_PERF_OPEN, struct drm_i915_perf_open_param) #define DRM_IOCTL_I915_PERF_ADD_CONFIG DRM_IOW(DRM_COMMAND_BASE + DRM_I915_PERF_ADD_CONFIG, struct drm_i915_perf_oa_config) #define DRM_IOCTL_I915_PERF_REMOVE_CONFIG DRM_IOW(DRM_COMMAND_BASE + DRM_I915_PERF_REMOVE_CONFIG, __u64) +#define DRM_IOCTL_I915_READ_SCRATCH_PAGE DRM_IOWR (DRM_COMMAND_BASE + DRM_I915_READ_SCRATCH_PAGE, struct drm_i915_scratch_page) +#define DRM_IOCTL_I915_WRITE_SCRATCH_PAGE DRM_IOWR (DRM_COMMAND_BASE + DRM_I915_WRITE_SCRATCH_PAGE, struct drm_i915_scratch_page) /* Allow drivers to submit batchbuffers directly to hardware, relying * on the security mechanisms provided by hardware. @@ -1535,6 +1539,33 @@ struct drm_i915_perf_oa_config { __u64 flex_regs_ptr; }; +/** + * Structure to read/write scratch page of PPGTT. Read and writing + * values are not reliable unless the calling application guarantees + * that no batchbuffer that could read or write the scratch is in + * flight using the PPGTT between the time the ioctl is issued and + * it returns. + */ +struct drm_i915_scratch_page { + /** + * size in bytes of the backing store pointed to by buffer_ptr; + * kernel will return the actual size of the scratch page in + * this field as well. + */ + __u32 buffer_size; + + /** + * Pointer data with which to upload to or download from the + * scratch page; if the buffer size behind buffer_ptr is + * smaller than the scratch page size, then only the first + * buffer_size bytes are read or written. If the scratch + * page size is greater than buffer_size, then the bytes + * past the scratch page size in buffer behind bufer_ptr + * are not read or writte. + */ + __u64 buffer_ptr; +}; + #if defined(__cplusplus) } #endif -- 2.15.0 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 1/3] drm-uapi: define interface to kernel for scratch page read 2017-12-05 7:48 ` [PATCH 1/3] drm-uapi: define interface to kernel for scratch page read kevin.rogovin @ 2017-12-05 7:57 ` Rogovin, Kevin 0 siblings, 0 replies; 11+ messages in thread From: Rogovin, Kevin @ 2017-12-05 7:57 UTC (permalink / raw) To: intel-gfx@lists.freedesktop.org Sighs, I do not know why git send-email made this into two threads, but there it is. Worse, the second patch was from an older version; the one posted lacks the write to the scratch page with noise. At any rate, the thing is also on github at https://github.com/krogueintel/asem/tree/out-of-bounds-write-detect which has the initialization of the scratch page with noise. -Kevin -----Original Message----- From: Rogovin, Kevin Sent: Tuesday, December 5, 2017 9:48 AM To: intel-gfx@lists.freedesktop.org Cc: Rogovin, Kevin <kevin.rogovin@intel.com> Subject: [PATCH 1/3] drm-uapi: define interface to kernel for scratch page read From: Kevin Rogovin <kevin.rogovin@intel.com> --- include/drm-uapi/i915_drm.h | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/include/drm-uapi/i915_drm.h b/include/drm-uapi/i915_drm.h index 890df227ae..3a9c3a2d0c 100644 --- a/include/drm-uapi/i915_drm.h +++ b/include/drm-uapi/i915_drm.h @@ -262,6 +262,8 @@ typedef struct _drm_i915_sarea { #define DRM_I915_PERF_OPEN 0x36 #define DRM_I915_PERF_ADD_CONFIG 0x37 #define DRM_I915_PERF_REMOVE_CONFIG 0x38 +#define DRM_I915_READ_SCRATCH_PAGE 0x39 +#define DRM_I915_WRITE_SCRATCH_PAGE 0x40 #define DRM_IOCTL_I915_INIT DRM_IOW( DRM_COMMAND_BASE + DRM_I915_INIT, drm_i915_init_t) #define DRM_IOCTL_I915_FLUSH DRM_IO ( DRM_COMMAND_BASE + DRM_I915_FLUSH) @@ -319,6 +321,8 @@ typedef struct _drm_i915_sarea { #define DRM_IOCTL_I915_PERF_OPEN DRM_IOW(DRM_COMMAND_BASE + DRM_I915_PERF_OPEN, struct drm_i915_perf_open_param) #define DRM_IOCTL_I915_PERF_ADD_CONFIG DRM_IOW(DRM_COMMAND_BASE + DRM_I915_PERF_ADD_CONFIG, struct drm_i915_perf_oa_config) #define DRM_IOCTL_I915_PERF_REMOVE_CONFIG DRM_IOW(DRM_COMMAND_BASE + DRM_I915_PERF_REMOVE_CONFIG, __u64) +#define DRM_IOCTL_I915_READ_SCRATCH_PAGE DRM_IOWR (DRM_COMMAND_BASE + DRM_I915_READ_SCRATCH_PAGE, struct drm_i915_scratch_page) +#define DRM_IOCTL_I915_WRITE_SCRATCH_PAGE DRM_IOWR (DRM_COMMAND_BASE + DRM_I915_WRITE_SCRATCH_PAGE, struct drm_i915_scratch_page) /* Allow drivers to submit batchbuffers directly to hardware, relying * on the security mechanisms provided by hardware. @@ -1535,6 +1539,33 @@ struct drm_i915_perf_oa_config { __u64 flex_regs_ptr; }; +/** + * Structure to read/write scratch page of PPGTT. Read and writing + * values are not reliable unless the calling application guarantees + * that no batchbuffer that could read or write the scratch is in + * flight using the PPGTT between the time the ioctl is issued and + * it returns. + */ +struct drm_i915_scratch_page { + /** + * size in bytes of the backing store pointed to by buffer_ptr; + * kernel will return the actual size of the scratch page in + * this field as well. + */ + __u32 buffer_size; + + /** + * Pointer data with which to upload to or download from the + * scratch page; if the buffer size behind buffer_ptr is + * smaller than the scratch page size, then only the first + * buffer_size bytes are read or written. If the scratch + * page size is greater than buffer_size, then the bytes + * past the scratch page size in buffer behind bufer_ptr + * are not read or writte. + */ + __u64 buffer_ptr; +}; + #if defined(__cplusplus) } #endif -- 2.15.0 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 2/3] i965: define stuff for scratch page checking in intel_screen 2017-12-05 7:48 [PATCH 0/3] RFC: Scratch page checking kevin.rogovin 2017-12-05 7:48 ` [PATCH 1/3] drm-uapi: define interface to kernel for scratch page read kevin.rogovin @ 2017-12-05 7:48 ` kevin.rogovin 2017-12-05 7:58 ` Rogovin, Kevin 2017-12-05 7:48 ` [PATCH 3/3] i965: check scratch page in a locked fashion on each ioctl kevin.rogovin 2 siblings, 1 reply; 11+ messages in thread From: kevin.rogovin @ 2017-12-05 7:48 UTC (permalink / raw) To: intel-gfx; +Cc: Kevin Rogovin From: Kevin Rogovin <kevin.rogovin@intel.com> --- src/intel/common/gen_debug.c | 1 + src/intel/common/gen_debug.h | 1 + src/mesa/drivers/dri/i965/intel_screen.c | 26 ++++++++++++++++++++++++++ src/mesa/drivers/dri/i965/intel_screen.h | 12 ++++++++++++ 4 files changed, 40 insertions(+) diff --git a/src/intel/common/gen_debug.c b/src/intel/common/gen_debug.c index f58c593c44..7bd6723311 100644 --- a/src/intel/common/gen_debug.c +++ b/src/intel/common/gen_debug.c @@ -84,6 +84,7 @@ static const struct debug_control debug_control[] = { { "nohiz", DEBUG_NO_HIZ }, { "color", DEBUG_COLOR }, { "reemit", DEBUG_REEMIT }, + { "check_scratch", DEBUG_CHECK_SCRATH }, { NULL, 0 } }; diff --git a/src/intel/common/gen_debug.h b/src/intel/common/gen_debug.h index e418e3fb16..5e224a45f0 100644 --- a/src/intel/common/gen_debug.h +++ b/src/intel/common/gen_debug.h @@ -83,6 +83,7 @@ extern uint64_t INTEL_DEBUG; #define DEBUG_NO_HIZ (1ull << 39) #define DEBUG_COLOR (1ull << 40) #define DEBUG_REEMIT (1ull << 41) +#define DEBUG_CHECK_SCRATH (1ull << 42) #ifdef HAVE_ANDROID_PLATFORM #define LOG_TAG "INTEL-MESA" diff --git a/src/mesa/drivers/dri/i965/intel_screen.c b/src/mesa/drivers/dri/i965/intel_screen.c index 38769babf0..044be8fe85 100644 --- a/src/mesa/drivers/dri/i965/intel_screen.c +++ b/src/mesa/drivers/dri/i965/intel_screen.c @@ -1557,6 +1557,12 @@ intelDestroyScreen(__DRIscreen * sPriv) brw_bufmgr_destroy(screen->bufmgr); driDestroyOptionInfo(&screen->optionCache); + if (screen->debug_batchbuffer.enabled) { + simple_mtx_destroy(&screen->debug_batchbuffer.mutex); + free(screen->debug_batchbuffer.noise_values); + free(screen->debug_batchbuffer.tmp); + } + ralloc_free(screen); sPriv->driverPrivate = NULL; } @@ -2610,6 +2616,26 @@ __DRIconfig **intelInitScreen2(__DRIscreen *dri_screen) } } + screen->debug_batchbuffer.enabled = false; + if (INTEL_DEBUG & DEBUG_CHECK_SCRATH) { + struct drm_i915_scratch_page sc; + int err; + + sc.buffer_size = 0; + sc.buffer_ptr = 0; + err = drmIoctl(dri_screen->fd, DRM_IOCTL_I915_READ_SCRATCH_PAGE, &sc); + if (err == 0) { + screen->debug_batchbuffer.enabled = true; + simple_mtx_init(&screen->debug_batchbuffer.mutex, mtx_plain); + screen->debug_batchbuffer.buffer_size = sc.buffer_size; + screen->debug_batchbuffer.noise_values = calloc(screen->debug_batchbuffer.buffer_size, 1); + screen->debug_batchbuffer.tmp = calloc(screen->debug_batchbuffer.buffer_size, 1); + for (uint64_t i = 0; i < screen->debug_batchbuffer.buffer_size; ++i) { + screen->debug_batchbuffer.noise_values[i] = rand() & 0xFF; + } + } + } + return (const __DRIconfig**) intel_screen_make_configs(dri_screen); } diff --git a/src/mesa/drivers/dri/i965/intel_screen.h b/src/mesa/drivers/dri/i965/intel_screen.h index 7948617b7f..7d56106aa2 100644 --- a/src/mesa/drivers/dri/i965/intel_screen.h +++ b/src/mesa/drivers/dri/i965/intel_screen.h @@ -37,6 +37,7 @@ #include "common/gen_device_info.h" #include "i915_drm.h" #include "util/xmlconfig.h" +#include "util/simple_mtx.h" #include "isl/isl.h" @@ -114,6 +115,17 @@ struct intel_screen */ int eu_total; + /** + * Struct to perform out-of-bound GEM BO write checking + */ + struct { + bool enabled; + simple_mtx_t mutex; + uint32_t buffer_size; + uint8_t *noise_values; + uint8_t *tmp; + } debug_batchbuffer; + bool mesa_format_supports_texture[MESA_FORMAT_COUNT]; bool mesa_format_supports_render[MESA_FORMAT_COUNT]; enum isl_format mesa_to_isl_render_format[MESA_FORMAT_COUNT]; -- 2.15.0 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 2/3] i965: define stuff for scratch page checking in intel_screen 2017-12-05 7:48 ` [PATCH 2/3] i965: define stuff for scratch page checking in intel_screen kevin.rogovin @ 2017-12-05 7:58 ` Rogovin, Kevin 0 siblings, 0 replies; 11+ messages in thread From: Rogovin, Kevin @ 2017-12-05 7:58 UTC (permalink / raw) To: intel-gfx@lists.freedesktop.org Sighs, I do not know why git send-email made this into two threads, but there it is. Worse, the second patch was from an older version; the one posted lacks the write to the scratch page with noise. At any rate, the thing is also on github at https://github.com/krogueintel/asem/tree/out-of-bounds-write-detect which has the initialization of the scratch page with noise. -Kevin -----Original Message----- From: Rogovin, Kevin Sent: Tuesday, December 5, 2017 9:48 AM To: intel-gfx@lists.freedesktop.org Cc: Rogovin, Kevin <kevin.rogovin@intel.com> Subject: [PATCH 2/3] i965: define stuff for scratch page checking in intel_screen From: Kevin Rogovin <kevin.rogovin@intel.com> --- src/intel/common/gen_debug.c | 1 + src/intel/common/gen_debug.h | 1 + src/mesa/drivers/dri/i965/intel_screen.c | 26 ++++++++++++++++++++++++++ src/mesa/drivers/dri/i965/intel_screen.h | 12 ++++++++++++ 4 files changed, 40 insertions(+) diff --git a/src/intel/common/gen_debug.c b/src/intel/common/gen_debug.c index f58c593c44..7bd6723311 100644 --- a/src/intel/common/gen_debug.c +++ b/src/intel/common/gen_debug.c @@ -84,6 +84,7 @@ static const struct debug_control debug_control[] = { { "nohiz", DEBUG_NO_HIZ }, { "color", DEBUG_COLOR }, { "reemit", DEBUG_REEMIT }, + { "check_scratch", DEBUG_CHECK_SCRATH }, { NULL, 0 } }; diff --git a/src/intel/common/gen_debug.h b/src/intel/common/gen_debug.h index e418e3fb16..5e224a45f0 100644 --- a/src/intel/common/gen_debug.h +++ b/src/intel/common/gen_debug.h @@ -83,6 +83,7 @@ extern uint64_t INTEL_DEBUG; #define DEBUG_NO_HIZ (1ull << 39) #define DEBUG_COLOR (1ull << 40) #define DEBUG_REEMIT (1ull << 41) +#define DEBUG_CHECK_SCRATH (1ull << 42) #ifdef HAVE_ANDROID_PLATFORM #define LOG_TAG "INTEL-MESA" diff --git a/src/mesa/drivers/dri/i965/intel_screen.c b/src/mesa/drivers/dri/i965/intel_screen.c index 38769babf0..044be8fe85 100644 --- a/src/mesa/drivers/dri/i965/intel_screen.c +++ b/src/mesa/drivers/dri/i965/intel_screen.c @@ -1557,6 +1557,12 @@ intelDestroyScreen(__DRIscreen * sPriv) brw_bufmgr_destroy(screen->bufmgr); driDestroyOptionInfo(&screen->optionCache); + if (screen->debug_batchbuffer.enabled) { + simple_mtx_destroy(&screen->debug_batchbuffer.mutex); + free(screen->debug_batchbuffer.noise_values); + free(screen->debug_batchbuffer.tmp); + } + ralloc_free(screen); sPriv->driverPrivate = NULL; } @@ -2610,6 +2616,26 @@ __DRIconfig **intelInitScreen2(__DRIscreen *dri_screen) } } + screen->debug_batchbuffer.enabled = false; + if (INTEL_DEBUG & DEBUG_CHECK_SCRATH) { + struct drm_i915_scratch_page sc; + int err; + + sc.buffer_size = 0; + sc.buffer_ptr = 0; + err = drmIoctl(dri_screen->fd, DRM_IOCTL_I915_READ_SCRATCH_PAGE, &sc); + if (err == 0) { + screen->debug_batchbuffer.enabled = true; + simple_mtx_init(&screen->debug_batchbuffer.mutex, mtx_plain); + screen->debug_batchbuffer.buffer_size = sc.buffer_size; + screen->debug_batchbuffer.noise_values = calloc(screen->debug_batchbuffer.buffer_size, 1); + screen->debug_batchbuffer.tmp = calloc(screen->debug_batchbuffer.buffer_size, 1); + for (uint64_t i = 0; i < screen->debug_batchbuffer.buffer_size; ++i) { + screen->debug_batchbuffer.noise_values[i] = rand() & 0xFF; + } + } + } + return (const __DRIconfig**) intel_screen_make_configs(dri_screen); } diff --git a/src/mesa/drivers/dri/i965/intel_screen.h b/src/mesa/drivers/dri/i965/intel_screen.h index 7948617b7f..7d56106aa2 100644 --- a/src/mesa/drivers/dri/i965/intel_screen.h +++ b/src/mesa/drivers/dri/i965/intel_screen.h @@ -37,6 +37,7 @@ #include "common/gen_device_info.h" #include "i915_drm.h" #include "util/xmlconfig.h" +#include "util/simple_mtx.h" #include "isl/isl.h" @@ -114,6 +115,17 @@ struct intel_screen */ int eu_total; + /** + * Struct to perform out-of-bound GEM BO write checking + */ + struct { + bool enabled; + simple_mtx_t mutex; + uint32_t buffer_size; + uint8_t *noise_values; + uint8_t *tmp; + } debug_batchbuffer; + bool mesa_format_supports_texture[MESA_FORMAT_COUNT]; bool mesa_format_supports_render[MESA_FORMAT_COUNT]; enum isl_format mesa_to_isl_render_format[MESA_FORMAT_COUNT]; -- 2.15.0 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 3/3] i965: check scratch page in a locked fashion on each ioctl 2017-12-05 7:48 [PATCH 0/3] RFC: Scratch page checking kevin.rogovin 2017-12-05 7:48 ` [PATCH 1/3] drm-uapi: define interface to kernel for scratch page read kevin.rogovin 2017-12-05 7:48 ` [PATCH 2/3] i965: define stuff for scratch page checking in intel_screen kevin.rogovin @ 2017-12-05 7:48 ` kevin.rogovin 2017-12-05 10:07 ` Chris Wilson 2 siblings, 1 reply; 11+ messages in thread From: kevin.rogovin @ 2017-12-05 7:48 UTC (permalink / raw) To: intel-gfx; +Cc: Kevin Rogovin From: Kevin Rogovin <kevin.rogovin@intel.com> --- src/mesa/drivers/dri/i965/intel_batchbuffer.c | 27 ++++++++++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) diff --git a/src/mesa/drivers/dri/i965/intel_batchbuffer.c b/src/mesa/drivers/dri/i965/intel_batchbuffer.c index 216073129b..53b3eaf49b 100644 --- a/src/mesa/drivers/dri/i965/intel_batchbuffer.c +++ b/src/mesa/drivers/dri/i965/intel_batchbuffer.c @@ -804,7 +804,8 @@ static int submit_batch(struct brw_context *brw, int in_fence_fd, int *out_fence_fd) { const struct gen_device_info *devinfo = &brw->screen->devinfo; - __DRIscreen *dri_screen = brw->screen->driScrnPriv; + struct intel_screen *screen = brw->screen; + __DRIscreen *dri_screen = screen->driScrnPriv; struct intel_batchbuffer *batch = &brw->batch; int ret = 0; @@ -875,10 +876,34 @@ submit_batch(struct brw_context *brw, int in_fence_fd, int *out_fence_fd) batch->validation_list[index] = tmp; } + if (unlikely(screen->debug_batchbuffer.enabled)) { + simple_mtx_lock(&screen->debug_batchbuffer.mutex); + } + ret = execbuffer(dri_screen->fd, batch, hw_ctx, 4 * USED_BATCH(*batch), in_fence_fd, out_fence_fd, flags); + if (unlikely(screen->debug_batchbuffer.enabled)) { + struct drm_i915_scratch_page sc; + int ret; + + while (brw_bo_busy(batch->bo)) { + usleep(10); + } + + sc.buffer_size = screen->debug_batchbuffer.buffer_size; + sc.buffer_ptr = (__u64)(uintptr_t) screen->debug_batchbuffer.tmp; + + ret = drmIoctl(dri_screen->fd, DRM_IOCTL_I915_READ_SCRATCH_PAGE, &sc); + assert(ret == 0); + assert(sc.buffer_size == screen->debug_batchbuffer.buffer_size); + assert(memcmp(screen->debug_batchbuffer.tmp, + screen->debug_batchbuffer.noise_values, + screen->debug_batchbuffer.buffer_size) == 0); + simple_mtx_unlock(&screen->debug_batchbuffer.mutex); + } + throttle(brw); } -- 2.15.0 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 3/3] i965: check scratch page in a locked fashion on each ioctl 2017-12-05 7:48 ` [PATCH 3/3] i965: check scratch page in a locked fashion on each ioctl kevin.rogovin @ 2017-12-05 10:07 ` Chris Wilson 2017-12-05 10:30 ` Rogovin, Kevin 0 siblings, 1 reply; 11+ messages in thread From: Chris Wilson @ 2017-12-05 10:07 UTC (permalink / raw) To: intel-gfx; +Cc: Kevin Rogovin Quoting kevin.rogovin@intel.com (2017-12-05 07:48:14) > From: Kevin Rogovin <kevin.rogovin@intel.com> > > --- > src/mesa/drivers/dri/i965/intel_batchbuffer.c | 27 ++++++++++++++++++++++++++- > 1 file changed, 26 insertions(+), 1 deletion(-) > > diff --git a/src/mesa/drivers/dri/i965/intel_batchbuffer.c b/src/mesa/drivers/dri/i965/intel_batchbuffer.c > index 216073129b..53b3eaf49b 100644 > --- a/src/mesa/drivers/dri/i965/intel_batchbuffer.c > +++ b/src/mesa/drivers/dri/i965/intel_batchbuffer.c > @@ -804,7 +804,8 @@ static int > submit_batch(struct brw_context *brw, int in_fence_fd, int *out_fence_fd) > { > const struct gen_device_info *devinfo = &brw->screen->devinfo; > - __DRIscreen *dri_screen = brw->screen->driScrnPriv; > + struct intel_screen *screen = brw->screen; > + __DRIscreen *dri_screen = screen->driScrnPriv; > struct intel_batchbuffer *batch = &brw->batch; > int ret = 0; > > @@ -875,10 +876,34 @@ submit_batch(struct brw_context *brw, int in_fence_fd, int *out_fence_fd) > batch->validation_list[index] = tmp; > } > > + if (unlikely(screen->debug_batchbuffer.enabled)) { > + simple_mtx_lock(&screen->debug_batchbuffer.mutex); > + } Per context, then you can remove the locking. It's fitting since the scratch page is per-context anyway. > + > ret = execbuffer(dri_screen->fd, batch, hw_ctx, > 4 * USED_BATCH(*batch), > in_fence_fd, out_fence_fd, flags); > > + if (unlikely(screen->debug_batchbuffer.enabled)) { > + struct drm_i915_scratch_page sc; > + int ret; Tie this into INTEL_DEBUG & SYNC, then you can do all the synchronous operations in one place. -Chris _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 3/3] i965: check scratch page in a locked fashion on each ioctl 2017-12-05 10:07 ` Chris Wilson @ 2017-12-05 10:30 ` Rogovin, Kevin 2017-12-05 10:39 ` Chris Wilson 0 siblings, 1 reply; 11+ messages in thread From: Rogovin, Kevin @ 2017-12-05 10:30 UTC (permalink / raw) To: Chris Wilson, intel-gfx@lists.freedesktop.org Hi, > Per context, then you can remove the locking. It's fitting since the scratch page is per-context anyway. The scratch page is per context? This I did not know, I thought it was per PPGTT. If that is the case, then my proposed interface to get/set the scratch page contents is wrong because it does not pass the HW context id. -Kevin _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 3/3] i965: check scratch page in a locked fashion on each ioctl 2017-12-05 10:30 ` Rogovin, Kevin @ 2017-12-05 10:39 ` Chris Wilson 2017-12-05 10:41 ` Rogovin, Kevin 0 siblings, 1 reply; 11+ messages in thread From: Chris Wilson @ 2017-12-05 10:39 UTC (permalink / raw) To: Rogovin, Kevin, intel-gfx@lists.freedesktop.org Quoting Rogovin, Kevin (2017-12-05 10:30:04) > Hi, > > > Per context, then you can remove the locking. It's fitting since the scratch page is per-context anyway. > > The scratch page is per context? This I did not know, I thought it was per PPGTT. If that is the case, then my proposed interface to get/set the scratch page contents is wrong because it does not pass the HW context id. Yes, it per-vm, which is per-ctx on everything you want to investigate on. gen4-7 it is a global GTT with a global scratch, and just a mutex inside one process is not going to give you atomicity. -Chris _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 3/3] i965: check scratch page in a locked fashion on each ioctl 2017-12-05 10:39 ` Chris Wilson @ 2017-12-05 10:41 ` Rogovin, Kevin 2017-12-05 13:59 ` Joonas Lahtinen 0 siblings, 1 reply; 11+ messages in thread From: Rogovin, Kevin @ 2017-12-05 10:41 UTC (permalink / raw) To: Chris Wilson, intel-gfx@lists.freedesktop.org Thanks, I will make a v2 and post it shortly to correct for my terribly embarrassing omission caused by even more terribly embarrassing ignorance. -Kevin -----Original Message----- From: Chris Wilson [mailto:chris@chris-wilson.co.uk] Sent: Tuesday, December 5, 2017 12:39 PM To: Rogovin, Kevin <kevin.rogovin@intel.com>; intel-gfx@lists.freedesktop.org Subject: RE: [Intel-gfx] [PATCH 3/3] i965: check scratch page in a locked fashion on each ioctl Quoting Rogovin, Kevin (2017-12-05 10:30:04) > Hi, > > > Per context, then you can remove the locking. It's fitting since the scratch page is per-context anyway. > > The scratch page is per context? This I did not know, I thought it was per PPGTT. If that is the case, then my proposed interface to get/set the scratch page contents is wrong because it does not pass the HW context id. Yes, it per-vm, which is per-ctx on everything you want to investigate on. gen4-7 it is a global GTT with a global scratch, and just a mutex inside one process is not going to give you atomicity. -Chris _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 3/3] i965: check scratch page in a locked fashion on each ioctl 2017-12-05 10:41 ` Rogovin, Kevin @ 2017-12-05 13:59 ` Joonas Lahtinen 0 siblings, 0 replies; 11+ messages in thread From: Joonas Lahtinen @ 2017-12-05 13:59 UTC (permalink / raw) To: Rogovin, Kevin, Chris Wilson, intel-gfx@lists.freedesktop.org On Tue, 2017-12-05 at 10:41 +0000, Rogovin, Kevin wrote: > Thanks, I will make a v2 and post it shortly to correct for my > terribly embarrassing omission caused by even more terribly > embarrassing ignorance. To avoid v3, do concept the code around suggested existing I915_GEM_CONTEXT_GETPARAM and I915_GEM_CONTEXT_SETPARAM ioctls. Just add a new parameter I915_CONTEXT_PARAM_SCRATCH_PAGE. The interface should become pretty bullet-proof that way. Regards, Joonas > > -Kevin > > -----Original Message----- > From: Chris Wilson [mailto:chris@chris-wilson.co.uk] > Sent: Tuesday, December 5, 2017 12:39 PM > To: Rogovin, Kevin <kevin.rogovin@intel.com>; intel-gfx@lists.freedes > ktop.org > Subject: RE: [Intel-gfx] [PATCH 3/3] i965: check scratch page in a > locked fashion on each ioctl > > Quoting Rogovin, Kevin (2017-12-05 10:30:04) > > Hi, > > > > > Per context, then you can remove the locking. It's fitting since > > > the scratch page is per-context anyway. > > > > The scratch page is per context? This I did not know, I thought it > > was per PPGTT. If that is the case, then my proposed interface to > > get/set the scratch page contents is wrong because it does not pass > > the HW context id. > > Yes, it per-vm, which is per-ctx on everything you want to > investigate on. gen4-7 it is a global GTT with a global scratch, and > just a mutex inside one process is not going to give you atomicity. > -Chris > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Joonas Lahtinen Open Source Technology Center Intel Corporation _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2017-12-05 13:59 UTC | newest] Thread overview: 11+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2017-12-05 7:48 [PATCH 0/3] RFC: Scratch page checking kevin.rogovin 2017-12-05 7:48 ` [PATCH 1/3] drm-uapi: define interface to kernel for scratch page read kevin.rogovin 2017-12-05 7:57 ` Rogovin, Kevin 2017-12-05 7:48 ` [PATCH 2/3] i965: define stuff for scratch page checking in intel_screen kevin.rogovin 2017-12-05 7:58 ` Rogovin, Kevin 2017-12-05 7:48 ` [PATCH 3/3] i965: check scratch page in a locked fashion on each ioctl kevin.rogovin 2017-12-05 10:07 ` Chris Wilson 2017-12-05 10:30 ` Rogovin, Kevin 2017-12-05 10:39 ` Chris Wilson 2017-12-05 10:41 ` Rogovin, Kevin 2017-12-05 13:59 ` Joonas Lahtinen
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.