On Thu, Jul 27, 2023 at 03:55:01PM +0100, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
>
> Now that i915 understands the caching modes behind PAT indices, we can
> refine the check in vm_fault_gtt() to not reject the uncached PAT if it
> was set by userspace on a snoopable platform.
>
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
> Cc: Fei Yang <fei.yang@xxxxxxxxx>
> Cc: Matt Roper <matthew.d.roper@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/gem/i915_gem_mman.c | 14 +++-----------
> 1 file changed, 3 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_mman.c b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
> index cd7f8ded0d6f..9aa6ecf68432 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_mman.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
> @@ -382,17 +382,9 @@ static vm_fault_t vm_fault_gtt(struct vm_fault *vmf)
> goto err_reset;
> }
>
> - /*
> - * For objects created by userspace through GEM_CREATE with pat_index
> - * set by set_pat extension, coherency is managed by userspace, make
> - * sure we don't fail handling the vm fault by calling
> - * i915_gem_object_has_cache_level() which always return true for such
> - * objects. Otherwise this helper function would fall back to checking
> - * whether the object is un-cached.
> - */
> - if (!((obj->pat_set_by_user ||
> - i915_gem_object_has_cache_mode(obj, I915_CACHE_MODE_UC)) ||
> - HAS_LLC(i915))) {
> + /* Access to snoopable pages through the GTT is incoherent. */
This comment was removed in the previous patch, but now it came back
here. Should we have just left it be in the previous patch?
I'm not really clear on what it means either. Are we using "GTT" as
shorthand to refer to the aperture here?
Matt
> + if (!i915_gem_object_has_cache_mode(obj, I915_CACHE_MODE_UC) &&
> + !HAS_LLC(i915)) {
> ret = -EFAULT;
> goto err_unpin;
> }
> --
> 2.39.2
>
--
Matt Roper
Graphics Software Engineer
Linux GPU Platform Enablement
Intel Corporation
