<div dir="ltr"> <div class="gmail_quote"><div dir="ltr">On Wed, Oct 17, 2018 at 6:47 PM Rafael Antognolli <<a href="mailto:rafael.antognolli@intel.com">rafael.antognolli@intel.com</a>> wrote: </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">On Wed, Oct 17, 2018 at 06:08:34PM +0300, Danylo Piliaiev wrote: > Signed-off-by: Danylo Piliaiev <<a href="mailto:danylo.piliaiev@globallogic.com" target="_blank">danylo.piliaiev@globallogic.com</a>> > --- > src/intel/tools/intel_sanitize_gpu.c | 38 +++++++++++++++------------- > 1 file changed, 20 insertions(+), 18 deletions(-) > > diff --git a/src/intel/tools/intel_sanitize_gpu.c b/src/intel/tools/intel_sanitize_gpu.c > index 9b49b0bbf2..36c4725a2f 100644 > --- a/src/intel/tools/intel_sanitize_gpu.c > +++ b/src/intel/tools/intel_sanitize_gpu.c > @@ -51,14 +51,6 @@ static int (*libc_fcntl)(int fd, int cmd, int param); > > #define DRM_MAJOR 226 > > -/* 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 BO. Indeed, > - * depending on GPU, kernel version and GEM size, the > - * page size can be one of 4KB, 64KB or 2M. > - */ > -#define PADDING_SIZE 4096 > - > struct refcnt_hash_table { > struct hash_table *t; > int refcnt; > @@ -80,6 +72,8 @@ pthread_mutex_t mutex = PTHREAD_MUTEX_INITIALIZER; > > static struct hash_table *fds_to_bo_sizes = NULL; > > +static long padding_size = 0; > + > static inline struct hash_table* > bo_size_table(int fd) > { > @@ -166,7 +160,7 @@ padding_is_good(int fd, uint32_t handle) > struct drm_i915_gem_mmap mmap_arg = { > .handle = handle, > .offset = bo_size(fd, handle), > - .size = PADDING_SIZE, > + .size = padding_size, > .flags = 0, > }; > > @@ -189,17 +183,17 @@ padding_is_good(int fd, uint32_t handle) > * if the bo is not cache coherent we likely need to > * invalidate the cache lines to get it. > */ > - gen_invalidate_range(mapped, PADDING_SIZE); > + gen_invalidate_range(mapped, padding_size); > > expected_value = handle & 0xFF; > - for (uint32_t i = 0; i < PADDING_SIZE; ++i) { > + for (uint32_t i = 0; i < padding_size; ++i) { > if (expected_value != mapped[i]) { > - munmap(mapped, PADDING_SIZE); > + munmap(mapped, padding_size); > return false; > } > expected_value = next_noise_value(expected_value); > } > - munmap(mapped, PADDING_SIZE); > + munmap(mapped, padding_size); > > return true; > } > @@ -207,9 +201,9 @@ padding_is_good(int fd, uint32_t handle) > static int > create_with_padding(int fd, struct drm_i915_gem_create *create) > { > - create->size += PADDING_SIZE; > + create->size += padding_size; > int ret = libc_ioctl(fd, DRM_IOCTL_I915_GEM_CREATE, create); > - create->size -= PADDING_SIZE; > + create->size -= padding_size; > > if (ret != 0) > return ret; > @@ -218,7 +212,7 @@ create_with_padding(int fd, struct drm_i915_gem_create *create) > struct drm_i915_gem_mmap mmap_arg = { > .handle = create->handle, > .offset = create->size, > - .size = PADDING_SIZE, > + .size = padding_size, > .flags = 0, > }; > > @@ -228,8 +222,8 @@ create_with_padding(int fd, struct drm_i915_gem_create *create) > > noise_values = (uint8_t*) (uintptr_t) mmap_arg.addr_ptr; > fill_noise_buffer(noise_values, create->handle & 0xFF, > - PADDING_SIZE); > - munmap(noise_values, PADDING_SIZE); > + padding_size); > + munmap(noise_values, padding_size); > > _mesa_hash_table_insert(bo_size_table(fd), (void*)(uintptr_t)create->handle, > (void*)(uintptr_t)create->size); > @@ -427,4 +421,12 @@ init(void) > libc_close = dlsym(RTLD_NEXT, "close"); > libc_fcntl = dlsym(RTLD_NEXT, "fcntl"); > libc_ioctl = dlsym(RTLD_NEXT, "ioctl"); > + > + /* We want to make sure that the padding forces > + * the BO to take another page on the (PP)GTT. > + */ > + padding_size = sysconf(_SC_PAGESIZE); I don't think this is the page size we want. This is the page size of CPU/system memory, which might be different from what the GPU is using to map pages. For instance, even if we are using 64K pages for GPU mapping, I think this call would still return 4K. Though I'm not sure if there's an interface to query the kernel which page size we are using for the GPU... </blockquote><div> </div><div>I looked a second time at the kernel code and indeed I somehow missed this, The page size may be even different between allocation and easily different from CPU page size. My patch makes no sense, sorry.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> > + if (padding_size == -1) { > + unreachable("Bad page size"); > + } > } > -- > 2.18.0 > > _______________________________________________ > mesa-dev mailing list > <a href="mailto:mesa-dev@lists.freedesktop.org" target="_blank">mesa-dev@lists.freedesktop.org</a> > <a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev" rel="noreferrer" target="_blank">https://lists.freedesktop.org/mailman/listinfo/mesa-dev</a> </blockquote></div></div>