Re: [Intel-gfx] [PATCH 3/9] drm/i915: extract bypass-llc check into helper

2021-10-20 Thread Thomas Hellström
On Mon, 2021-10-18 at 18:45 +0100, Matthew Auld wrote:
> It looks like we will need this in some more places, so extract as a
> helper.
> 
> Signed-off-by: Matthew Auld 
> Cc: Thomas Hellström 
> ---
>  drivers/gpu/drm/i915/gem/i915_gem_object.c | 26
> ++
>  drivers/gpu/drm/i915/gem/i915_gem_object.h |  1 +
>  drivers/gpu/drm/i915/gem/i915_gem_shmem.c  | 17 +-
>  3 files changed, 28 insertions(+), 16 deletions(-)
> 
Reviewed-by: Thomas Hellström 




[Intel-gfx] [PATCH 3/9] drm/i915: extract bypass-llc check into helper

2021-10-18 Thread Matthew Auld
It looks like we will need this in some more places, so extract as a
helper.

Signed-off-by: Matthew Auld 
Cc: Thomas Hellström 
---
 drivers/gpu/drm/i915/gem/i915_gem_object.c | 26 ++
 drivers/gpu/drm/i915/gem/i915_gem_object.h |  1 +
 drivers/gpu/drm/i915/gem/i915_gem_shmem.c  | 17 +-
 3 files changed, 28 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.c 
b/drivers/gpu/drm/i915/gem/i915_gem_object.c
index 76ce6a1500bc..1e426a42a36c 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_object.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_object.c
@@ -128,6 +128,32 @@ void i915_gem_object_set_cache_coherency(struct 
drm_i915_gem_object *obj,
!(obj->cache_coherent & I915_BO_CACHE_COHERENT_FOR_WRITE);
 }
 
+bool i915_gem_object_can_bypass_llc(struct drm_i915_gem_object *obj)
+{
+   struct drm_i915_private *i915 = to_i915(obj->base.dev);
+
+   /*
+* This is purely from a security perspective, so we simply don't care
+* about non-userspace objects being able to bypass the LLC.
+*/
+   if (!(obj->flags & I915_BO_ALLOC_USER))
+   return false;
+
+   /*
+* EHL and JSL add the 'Bypass LLC' MOCS entry, which should make it
+* possible for userspace to bypass the GTT caching bits set by the
+* kernel, as per the given object cache_level. This is troublesome
+* since the heavy flush we apply when first gathering the pages is
+* skipped if the kernel thinks the object is coherent with the GPU. As
+* a result it might be possible to bypass the cache and read the
+* contents of the page directly, which could be stale data. If it's
+* just a case of userspace shooting themselves in the foot then so be
+* it, but since i915 takes the stance of always zeroing memory before
+* handing it to userspace, we need to prevent this.
+*/
+   return IS_JSL_EHL(i915);
+}
+
 static void i915_gem_close_object(struct drm_gem_object *gem, struct drm_file 
*file)
 {
struct drm_i915_gem_object *obj = to_intel_bo(gem);
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.h 
b/drivers/gpu/drm/i915/gem/i915_gem_object.h
index 9df3ee60604e..59201801cec5 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_object.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_object.h
@@ -514,6 +514,7 @@ i915_gem_object_finish_access(struct drm_i915_gem_object 
*obj)
 
 void i915_gem_object_set_cache_coherency(struct drm_i915_gem_object *obj,
 unsigned int cache_level);
+bool i915_gem_object_can_bypass_llc(struct drm_i915_gem_object *obj);
 void i915_gem_object_flush_if_display(struct drm_i915_gem_object *obj);
 void i915_gem_object_flush_if_display_locked(struct drm_i915_gem_object *obj);
 
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c 
b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
index 11f072193f3b..cf11aa7e08a0 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
@@ -182,22 +182,7 @@ static int shmem_get_pages(struct drm_i915_gem_object *obj)
if (i915_gem_object_needs_bit17_swizzle(obj))
i915_gem_object_do_bit_17_swizzle(obj, st);
 
-   /*
-* EHL and JSL add the 'Bypass LLC' MOCS entry, which should make it
-* possible for userspace to bypass the GTT caching bits set by the
-* kernel, as per the given object cache_level. This is troublesome
-* since the heavy flush we apply when first gathering the pages is
-* skipped if the kernel thinks the object is coherent with the GPU. As
-* a result it might be possible to bypass the cache and read the
-* contents of the page directly, which could be stale data. If it's
-* just a case of userspace shooting themselves in the foot then so be
-* it, but since i915 takes the stance of always zeroing memory before
-* handing it to userspace, we need to prevent this.
-*
-* By setting cache_dirty here we make the clflush in set_pages
-* unconditional on such platforms.
-*/
-   if (IS_JSL_EHL(i915) && obj->flags & I915_BO_ALLOC_USER)
+   if (i915_gem_object_can_bypass_llc(obj))
obj->cache_dirty = true;
 
__i915_gem_object_set_pages(obj, st, sg_page_sizes);
-- 
2.26.3