> -----Original Message-----
> From: Auld, Matthew <matthew.a...@intel.com>
> Sent: Wednesday, July 27, 2022 10:14 PM
> To: intel-...@lists.freedesktop.org
> Cc: dri-devel@lists.freedesktop.org; Thomas Hellström 
> <thomas.hellst...@linux.intel.com>; C,
> Ramalingam <ramalinga...@intel.com>
> Subject: [PATCH] drm/i915/ttm: don't leak the ccs state
> 
> The kernel only manages the ccs state with lmem-only objects, however the 
> kernel should still take
> care not to leak the CCS state from the previous user.
> 
> Fixes: 48760ffe923a ("drm/i915/gt: Clear compress metadata for Flat-ccs 
> objects")
> Signed-off-by: Matthew Auld <matthew.a...@intel.com>
> Cc: Thomas Hellström <thomas.hellst...@linux.intel.com>
> Cc: Ramalingam C <ramalinga...@intel.com>
> ---
>  drivers/gpu/drm/i915/gt/intel_migrate.c | 23 ++++++++++++++++++++++-
>  1 file changed, 22 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_migrate.c 
> b/drivers/gpu/drm/i915/gt/intel_migrate.c
> index a69b244f14d0..9a0814422ba4 100644
> --- a/drivers/gpu/drm/i915/gt/intel_migrate.c
> +++ b/drivers/gpu/drm/i915/gt/intel_migrate.c
> @@ -708,7 +708,7 @@ intel_context_migrate_copy(struct intel_context *ce,
>       u8 src_access, dst_access;
>       struct i915_request *rq;
>       int src_sz, dst_sz;
> -     bool ccs_is_src;
> +     bool ccs_is_src, overwrite_ccs;
>       int err;
> 
>       GEM_BUG_ON(ce->vm != ce->engine->gt->migrate.context->vm);
> @@ -749,6 +749,8 @@ intel_context_migrate_copy(struct intel_context *ce,
>                       get_ccs_sg_sgt(&it_ccs, bytes_to_cpy);
>       }
> 
> +     overwrite_ccs = HAS_FLAT_CCS(i915) && !ccs_bytes_to_cpy &&
> +dst_is_lmem;
> +
>       src_offset = 0;
>       dst_offset = CHUNK_SZ;
>       if (HAS_64K_PAGES(ce->engine->i915)) { @@ -852,6 +854,25 @@
> intel_context_migrate_copy(struct intel_context *ce,
>                       if (err)
>                               goto out_rq;
>                       ccs_bytes_to_cpy -= ccs_sz;
> +             } else if (overwrite_ccs) {
> +                     err = rq->engine->emit_flush(rq, EMIT_INVALIDATE);
> +                     if (err)
> +                             goto out_rq;
> +
> +                     /*
> +                      * While we can't always restore/manage the CCS state,
> +                      * we still need to ensure we don't leak the CCS state
> +                      * from the previous user, so make sure we overwrite it
> +                      * with something.
> +                      */
> +                     err = emit_copy_ccs(rq, dst_offset, INDIRECT_ACCESS,
> +                                         dst_offset, DIRECT_ACCESS, len);
> +                     if (err)
> +                             goto out_rq;
> +
> +                     err = rq->engine->emit_flush(rq, EMIT_INVALIDATE);
> +                     if (err)
> +                             goto out_rq;
The change is looking good to the purpose. But shouldn't this be the part of 
lmem allocation itself?

Ram.
>               }
> 
>               /* Arbitration is re-enabled between requests. */
> --
> 2.37.1

Reply via email to