Looks Good Thanks, Reviewed-by: Ian Forbes <ian.forbes@xxxxxxxxxxxx> On Wed, Aug 14, 2024 at 2:28 PM Zack Rusin <zack.rusin@xxxxxxxxxxxx> wrote: > > The kms paths keep a persistent map active to read and compare the cursor > buffer. These maps can race with each other in simple scenario where: > a) buffer "a" mapped for update > b) buffer "a" mapped for compare > c) do the compare > d) unmap "a" for compare > e) update the cursor > f) unmap "a" for update > At step "e" the buffer has been unmapped and the read contents is bogus. > > Prevent unmapping of active read buffers by simply keeping a count of > how many paths have currently active maps and unmap only when the count > reaches 0. > > v2: Update doc strings > > Fixes: 485d98d472d5 ("drm/vmwgfx: Add support for CursorMob and CursorBypass 4") > Cc: Broadcom internal kernel review list <bcm-kernel-feedback-list@xxxxxxxxxxxx> > Cc: dri-devel@xxxxxxxxxxxxxxxxxxxxx > Cc: <stable@xxxxxxxxxxxxxxx> # v5.19+ > Signed-off-by: Zack Rusin <zack.rusin@xxxxxxxxxxxx> > --- > drivers/gpu/drm/vmwgfx/vmwgfx_bo.c | 13 +++++++++++-- > drivers/gpu/drm/vmwgfx/vmwgfx_bo.h | 3 +++ > 2 files changed, 14 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c > index f42ebc4a7c22..a0e433fbcba6 100644 > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c > @@ -360,6 +360,8 @@ void *vmw_bo_map_and_cache_size(struct vmw_bo *vbo, size_t size) > void *virtual; > int ret; > > + atomic_inc(&vbo->map_count); > + > virtual = ttm_kmap_obj_virtual(&vbo->map, ¬_used); > if (virtual) > return virtual; > @@ -383,11 +385,17 @@ void *vmw_bo_map_and_cache_size(struct vmw_bo *vbo, size_t size) > */ > void vmw_bo_unmap(struct vmw_bo *vbo) > { > + int map_count; > + > if (vbo->map.bo == NULL) > return; > > - ttm_bo_kunmap(&vbo->map); > - vbo->map.bo = NULL; > + map_count = atomic_dec_return(&vbo->map_count); > + > + if (!map_count) { > + ttm_bo_kunmap(&vbo->map); > + vbo->map.bo = NULL; > + } > } > > > @@ -421,6 +429,7 @@ static int vmw_bo_init(struct vmw_private *dev_priv, > vmw_bo->tbo.priority = 3; > vmw_bo->res_tree = RB_ROOT; > xa_init(&vmw_bo->detached_resources); > + atomic_set(&vmw_bo->map_count, 0); > > params->size = ALIGN(params->size, PAGE_SIZE); > drm_gem_private_object_init(vdev, &vmw_bo->tbo.base, params->size); > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.h b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.h > index 62b4342d5f7c..43b5439ec9f7 100644 > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.h > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.h > @@ -71,6 +71,8 @@ struct vmw_bo_params { > * @map: Kmap object for semi-persistent mappings > * @res_tree: RB tree of resources using this buffer object as a backing MOB > * @res_prios: Eviction priority counts for attached resources > + * @map_count: The number of currently active maps. Will differ from the > + * cpu_writers because it includes kernel maps. > * @cpu_writers: Number of synccpu write grabs. Protected by reservation when > * increased. May be decreased without reservation. > * @dx_query_ctx: DX context if this buffer object is used as a DX query MOB > @@ -90,6 +92,7 @@ struct vmw_bo { > u32 res_prios[TTM_MAX_BO_PRIORITY]; > struct xarray detached_resources; > > + atomic_t map_count; > atomic_t cpu_writers; > /* Not ref-counted. Protected by binding_mutex */ > struct vmw_resource *dx_query_ctx; > -- > 2.43.0 >