On Thu, 24 Oct 2024 15:54:32 +0100
Akash Goel <akash.goel@xxxxxxx> wrote:
> All CPU mappings are forced as uncached for Panthor buffer objects when
> system(IO) coherency is disabled. Physical backing for Panthor BOs is
> allocated by shmem, which clears the pages also after allocation. But
> there is no explicit cache flush done after the clearing of pages.
> So it could happen that there are dirty cachelines in the CPU cache
> for the BOs, when they are accessed from the CPU side through uncached
> CPU mapping, and the eviction of cachelines overwrites the data of BOs.
Hm, this looks like something that should be handled at the
drm_gem_shmem level when drm_gem_shmem_object::map_wc=true, as I
suspect other drivers can hit the same issue (I'm thinking of panfrost
and lima, but there might be others).
>
> This commit tries to avoid the potential overwrite scenario.
>
> Signed-off-by: Akash Goel <akash.goel@xxxxxxx>
> ---
> drivers/gpu/drm/panthor/panthor_gem.h | 10 ++++++++++
> drivers/gpu/drm/panthor/panthor_mmu.c | 5 +++++
> 2 files changed, 15 insertions(+)
>
> diff --git a/drivers/gpu/drm/panthor/panthor_gem.h b/drivers/gpu/drm/panthor/panthor_gem.h
> index e43021cf6d45..4b0f43f1edf1 100644
> --- a/drivers/gpu/drm/panthor/panthor_gem.h
> +++ b/drivers/gpu/drm/panthor/panthor_gem.h
> @@ -46,6 +46,16 @@ struct panthor_gem_object {
>
> /** @flags: Combination of drm_panthor_bo_flags flags. */
> u32 flags;
> +
> + /**
> + * @cleaned: The buffer object pages have been cleaned.
> + *
> + * There could be dirty CPU cachelines for the pages of buffer object
> + * after allocation, as shmem will zero out the pages. The cachelines
> + * need to be cleaned if the pages are going to be accessed with an
> + * uncached CPU mapping.
> + */
> + bool cleaned;
> };
>
> /**
> diff --git a/drivers/gpu/drm/panthor/panthor_mmu.c b/drivers/gpu/drm/panthor/panthor_mmu.c
> index f522a116c1b1..d8cc9e7d064e 100644
> --- a/drivers/gpu/drm/panthor/panthor_mmu.c
> +++ b/drivers/gpu/drm/panthor/panthor_mmu.c
> @@ -1249,6 +1249,11 @@ static int panthor_vm_prepare_map_op_ctx(struct panthor_vm_op_ctx *op_ctx,
>
> op_ctx->map.sgt = sgt;
>
> + if (bo->base.map_wc && !bo->cleaned) {
> + dma_sync_sgtable_for_device(vm->ptdev->base.dev, sgt, DMA_TO_DEVICE);
> + bo->cleaned = true;
> + }
> +
> preallocated_vm_bo = drm_gpuvm_bo_create(&vm->base, &bo->base.base);
> if (!preallocated_vm_bo) {
> if (!bo->base.base.import_attach)
