[Mesa-dev] [PATCH v3 2/3] i965: add noise padding to buffer object and function to check if noise is correct
Rogovin, Kevin
kevin.rogovin at intel.com
Fri Jan 26 11:51:33 UTC 2018
Hi,
The main reason I went with map and invalidate is because the kernel on pread will only wait on execbuffer2 calls that declare they are going to write to the given GEM; there is the off-chance that a wild write might hit the padding of a GEM and the function contract is to check the padding. This is following the review comments from Chris Wilson.
-Kevin
From: Jason Ekstrand [mailto:jason at jlekstrand.net]
Sent: Friday, January 26, 2018 12:13 PM
To: Rogovin, Kevin <kevin.rogovin at intel.com>
Cc: ML mesa-dev <mesa-dev at lists.freedesktop.org>
Subject: Re: [Mesa-dev] [PATCH v3 2/3] i965: add noise padding to buffer object and function to check if noise is correct
On Fri, Jan 26, 2018 at 12:56 AM, <kevin.rogovin at intel.com<mailto:kevin.rogovin at intel.com>> wrote:
From: Kevin Rogovin <kevin.rogovin at intel.com<mailto:kevin.rogovin at intel.com>>
Signed-off-by: Kevin Rogovin <kevin.rogovin at intel.com<mailto:kevin.rogovin at intel.com>>
Reviewed-by: Chris Wilson <chris at chris-wilson.co.uk<mailto:chris at chris-wilson.co.uk>>
---
src/mesa/drivers/dri/i965/brw_bufmgr.c | 115 ++++++++++++++++++++++++++++++++-
src/mesa/drivers/dri/i965/brw_bufmgr.h | 13 ++++
2 files changed, 127 insertions(+), 1 deletion(-)
diff --git a/src/mesa/drivers/dri/i965/brw_bufmgr.c b/src/mesa/drivers/dri/i965/brw_bufmgr.c
index fb180289a0..7e50ba512e 100644
--- a/src/mesa/drivers/dri/i965/brw_bufmgr.c
+++ b/src/mesa/drivers/dri/i965/brw_bufmgr.c
@@ -220,6 +220,39 @@ bucket_for_size(struct brw_bufmgr *bufmgr, uint64_t size)
&bufmgr->cache_bucket[index] : NULL;
}
+/* Our goal is not to have noise good enough for cryto,
+ * but instead values that are unique-ish enough that
+ * it is incredibly unlikely that a buffer overwrite
+ * will produce the exact same values.
+ */
+static uint8_t
+next_noise_value(uint8_t prev_noise)
+{
+ uint32_t v = prev_noise;
+ return (v * 103u + 227u) & 0xFF;
+}
+
+static void
+fill_noise_buffer(uint8_t *dst, uint8_t start, uint32_t length)
+{
+ for(uint32_t i = 0; i < length; ++i) {
+ dst[i] = start;
+ start = next_noise_value(start);
+ }
+}
+
+static uint8_t*
+generate_noise_buffer(uint8_t start, uint32_t length)
+{
+ uint8_t *return_value;
+ return_value = malloc(length);
+ if (return_value) {
+ fill_noise_buffer(return_value, start, length);
+ }
+
+ return return_value;
+}
+
int
brw_bo_busy(struct brw_bo *bo)
{
@@ -367,7 +400,18 @@ retry:
bo->size = bo_size;
bo->idle = true;
- struct drm_i915_gem_create create = { .size = bo_size };
+ bo->padding_size = 0;
+ if (unlikely(INTEL_DEBUG & DEBUG_OUT_OF_BOUND_CHK)) {
+ /* TODO: we want to make sure that the padding forces
+ * the BO to take another page on the (PP)GTT; 4KB
+ * may or may not be the page size for the GEM. Indeed,
+ * depending on generation, kernel version and GEM size,
+ * the page size can be one of 4KB, 64KB or 2M.
+ */
+ bo->padding_size = 4096;
+ }
+
+ struct drm_i915_gem_create create = { .size = bo_size + bo->padding_size };
/* All new BOs we get from the kernel are zeroed, so we don't need to
* worry about that here.
@@ -378,6 +422,27 @@ retry:
goto err;
}
+ if (unlikely(bo->padding_size > 0)) {
+ uint8_t *noise_values;
+ struct drm_i915_gem_pwrite pwrite;
+
+ noise_values = generate_noise_buffer(create.handle, bo->padding_size);
+ if (!noise_values)
+ goto err_free;
+
+ pwrite.handle = create.handle;
+ pwrite.pad = 0;
+ pwrite.offset = bo_size;
+ pwrite.size = bo->padding_size;
+ pwrite.data_ptr = (__u64) (uintptr_t) noise_values;
+
+ ret = drmIoctl(bufmgr->fd, DRM_IOCTL_I915_GEM_PWRITE, &pwrite);
+ free(noise_values);
+
+ if (ret != 0)
+ goto err_free;
+ }
+
bo->gem_handle = create.handle;
bo->bufmgr = bufmgr;
@@ -424,6 +489,53 @@ err:
return NULL;
}
+bool
+brw_bo_padding_is_good(struct brw_bo *bo)
+{
+ if (bo->padding_size > 0) {
+ struct drm_i915_gem_mmap mmap_arg = {
+ .handle = bo->gem_handle,
+ .offset = bo->size,
+ .size = bo->padding_size,
+ .flags = 0,
+ };
+ uint8_t *mapped;
+ int ret;
+ uint8_t expected_value;
+
+ /* We cannot use brw_bo_map() because it maps precisely the range
+ * [0, bo->size) and we wish to map the range of the padding which
+ * is [bo->size, bo->size + bo->pading_size)
+ */
+ ret = drmIoctl(bo->bufmgr->fd, DRM_IOCTL_I915_GEM_MMAP, &mmap_arg);
+ if (ret != 0) {
+ DBG("Unable to map mapping buffer %d (%s) buffer for pad checking.\n",
+ bo->gem_handle, bo->name);
+ return false;
+ }
+
+ mapped = (uint8_t*) (uintptr_t) mmap_arg.addr_ptr;
+ /* bah-humbug, we need to see the latest contents and
+ * if the bo is not cache coherent we likely need to
+ * invalidate the cache lines to get it.
+ */
+ if (!bo->cache_coherent && !bo->bufmgr->has_llc) {
+ gen_invalidate_range(mapped, bo->padding_size);
+ }
You used pwrite before. Why not just use pread and save yourself the hassle?
+
+ expected_value = bo->gem_handle & 0xFF;
+ for (uint32_t i = 0; i < bo->padding_size; ++i) {
+ if (expected_value != mapped[i]) {
+ drm_munmap(mapped, bo->padding_size);
+ return false;
+ }
+ expected_value = next_noise_value(expected_value);
+ }
+ drm_munmap(mapped, bo->padding_size);
+ }
+ return true;
+}
+
struct brw_bo *
brw_bo_alloc(struct brw_bufmgr *bufmgr,
const char *name, uint64_t size, uint64_t alignment)
@@ -1157,6 +1269,7 @@ brw_bo_gem_create_from_prime_internal(struct brw_bufmgr *bufmgr, int prime_fd,
bo->name = "prime";
bo->reusable = false;
bo->external = true;
+ bo->padding_size = 0;
if (tiling_mode < 0) {
struct drm_i915_gem_get_tiling get_tiling = { .handle = bo->gem_handle };
diff --git a/src/mesa/drivers/dri/i965/brw_bufmgr.h b/src/mesa/drivers/dri/i965/brw_bufmgr.h
index a3745d6667..fcfd2a9f75 100644
--- a/src/mesa/drivers/dri/i965/brw_bufmgr.h
+++ b/src/mesa/drivers/dri/i965/brw_bufmgr.h
@@ -165,6 +165,12 @@ struct brw_bo {
* Boolean of whether this buffer is cache coherent
*/
bool cache_coherent;
+
+ /**
+ * Padding size of weak pseudo-random values; used to check for out of
+ * bounds buffer writing.
+ */
+ uint32_t padding_size;
};
#define BO_ALLOC_BUSY (1<<0)
@@ -346,6 +352,13 @@ uint32_t brw_bo_export_gem_handle(struct brw_bo *bo);
int brw_reg_read(struct brw_bufmgr *bufmgr, uint32_t offset,
uint64_t *result);
+/* maps the padding of the brw_bo and returns true if and only if the
+ * padding is the correct noise values. The caller must make sure that
+ * the GPU is not going write to the BO; the best way to do that is to
+ * call brw_bo_wait_rendering() on the batchbuffer.
+ */
+bool brw_bo_padding_is_good(struct brw_bo *bo);
+
/** @{ */
#if defined(__cplusplus)
--
2.15.1
_______________________________________________
mesa-dev mailing list
mesa-dev at lists.freedesktop.org<mailto:mesa-dev at lists.freedesktop.org>
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20180126/51da6879/attachment-0001.html>
More information about the mesa-dev
mailing list