Quoting Daniel Vetter (2019-11-12 09:09:06)
On Thu, Jul 18, 2019 at 4:54 PM Chris Wilson chris@chris-wilson.co.uk wrote:
Ensure that we flush any cache dirt out to main memory before the user changes the cache-level as they may elect to bypass the cache (even after declaring their access cache-coherent) via use of unprivileged MOCS.
Signed-off-by: Chris Wilson chris@chris-wilson.co.uk Cc: Joonas Lahtinen joonas.lahtinen@linux.intel.com Cc: stable@vger.kernel.org
drivers/gpu/drm/i915/gem/i915_gem_domain.c | 5 +++++ 1 file changed, 5 insertions(+)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_domain.c b/drivers/gpu/drm/i915/gem/i915_gem_domain.c index 2e3ce2a69653..5d41e769a428 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_domain.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_domain.c @@ -277,6 +277,11 @@ int i915_gem_object_set_cache_level(struct drm_i915_gem_object *obj,
list_for_each_entry(vma, &obj->vma.list, obj_link) vma->node.color = cache_level;
/* Flush any previous cache dirt in case of cache bypass */if (obj->cache_dirty & ~obj->cache_coherent)i915_gem_clflush_object(obj, I915_CLFLUSH_SYNC);I think writing out the bit logic instead of implicitly relying on the #defines would be much better, i.e. && !(cache_coherent & COHERENT_FOR_READ). Plus I think we only need to set cache_dirty = true if we don't flush here already, to avoid double flushing?
No. The mask is being updated, so you need to flush before you lose track. The cache is then cleared of the dirty bit so won't be flushed again until dirty and no longer coherent. We need to flag that the page is no longer coherent at the end of its lifetime (passing back to the system) to force the flush then. -Chris