Commit 95d1deb0 authored by Rob Clark's avatar Rob Clark
Browse files

drm/msm/gem: Add fenced vma unpin 



With userspace allocated iova (next patch), we can have a race condition
where userspace observes the fence completion and deletes the vma before
retire_submit() gets around to unpinning the vma.  To handle this, add a
fenced unpin which drops the refcount but tracks the fence, and update
msm_gem_vma_inuse() to check any previously unsignaled fences.

v2: Fix inuse underflow (duplicate unpin)
v3: Fix msm_job_run() vs submit_cleanup() race condition

Signed-off-by: default avatarRob Clark <robdclark@chromium.org>
Link: https://lore.kernel.org/r/20220411215849.297838-10-robdclark@gmail.com


Signed-off-by: default avatarRob Clark <robdclark@chromium.org>
parent 27674c66

drivers/gpu/drm/msm/msm_fence.c

+4 −2
Original line number Diff line number Diff line
@@ -15,6 +15,7 @@ msm_fence_context_alloc(struct drm_device *dev, volatile uint32_t *fenceptr, 
		const char *name)

{

	struct msm_fence_context *fctx;

	static int index = 0;



	fctx = kzalloc(sizeof(*fctx), GFP_KERNEL);

	if (!fctx)
@@ -23,6 +24,7 @@ msm_fence_context_alloc(struct drm_device *dev, volatile uint32_t *fenceptr, 
	fctx->dev = dev;

	strncpy(fctx->name, name, sizeof(fctx->name));

	fctx->context = dma_fence_context_alloc(1);

	fctx->index = index++;

	fctx->fenceptr = fenceptr;

	spin_lock_init(&fctx->spinlock);
@@ -34,7 +36,7 @@ void msm_fence_context_free(struct msm_fence_context *fctx) 
	kfree(fctx);

}



static inline bool fence_completed(struct msm_fence_context *fctx, uint32_t fence)

bool msm_fence_completed(struct msm_fence_context *fctx, uint32_t fence)

{

	/*

	 * Note: Check completed_fence first, as fenceptr is in a write-combine
@@ -76,7 +78,7 @@ static const char *msm_fence_get_timeline_name(struct dma_fence *fence) 
static bool msm_fence_signaled(struct dma_fence *fence)

{

	struct msm_fence *f = to_msm_fence(fence);

	return fence_completed(f->fctx, f->base.seqno);

	return msm_fence_completed(f->fctx, f->base.seqno);

}



static const struct dma_fence_ops msm_fence_ops = {

drivers/gpu/drm/msm/msm_fence.h

+3 −0
Original line number Diff line number Diff line
@@ -21,6 +21,8 @@ struct msm_fence_context { 
	char name[32];

	/** context: see dma_fence_context_alloc() */

	unsigned context;

	/** index: similar to context, but local to msm_fence_context's */

	unsigned index;



	/**

	 * last_fence:
@@ -56,6 +58,7 @@ struct msm_fence_context * msm_fence_context_alloc(struct drm_device *dev, 
		volatile uint32_t *fenceptr, const char *name);

void msm_fence_context_free(struct msm_fence_context *fctx);



bool msm_fence_completed(struct msm_fence_context *fctx, uint32_t fence);

void msm_update_fence(struct msm_fence_context *fctx, uint32_t fence);



struct dma_fence * msm_fence_alloc(struct msm_fence_context *fctx);

drivers/gpu/drm/msm/msm_gem.c

+1 −1
Original line number Diff line number Diff line
@@ -445,7 +445,7 @@ void msm_gem_unpin_vma_locked(struct drm_gem_object *obj, struct msm_gem_vma *vm 


	GEM_WARN_ON(!msm_gem_is_locked(obj));



	msm_gem_unmap_vma(vma->aspace, vma);

	msm_gem_unpin_vma(vma);



	msm_obj->pin_count--;

	GEM_WARN_ON(msm_obj->pin_count < 0);

drivers/gpu/drm/msm/msm_gem.h

+12 −2
Original line number Diff line number Diff line
@@ -49,6 +49,8 @@ struct msm_gem_address_space * 
msm_gem_address_space_create(struct msm_mmu *mmu, const char *name,

		u64 va_start, u64 size);



struct msm_fence_context;



struct msm_gem_vma {

	struct drm_mm_node node;

	uint64_t iova;
@@ -56,6 +58,9 @@ struct msm_gem_vma { 
	struct list_head list;    /* node in msm_gem_object::vmas */

	bool mapped;

	int inuse;

	uint32_t fence_mask;

	uint32_t fence[MSM_GPU_MAX_RINGS];

	struct msm_fence_context *fctx[MSM_GPU_MAX_RINGS];

};



int msm_gem_init_vma(struct msm_gem_address_space *aspace,
@@ -64,8 +69,8 @@ int msm_gem_init_vma(struct msm_gem_address_space *aspace, 
bool msm_gem_vma_inuse(struct msm_gem_vma *vma);

void msm_gem_purge_vma(struct msm_gem_address_space *aspace,

		struct msm_gem_vma *vma);

void msm_gem_unmap_vma(struct msm_gem_address_space *aspace,

		struct msm_gem_vma *vma);

void msm_gem_unpin_vma(struct msm_gem_vma *vma);

void msm_gem_unpin_vma_fenced(struct msm_gem_vma *vma, struct msm_fence_context *fctx);

int msm_gem_map_vma(struct msm_gem_address_space *aspace,

		struct msm_gem_vma *vma, int prot,

		struct sg_table *sgt, int size);
@@ -363,6 +368,11 @@ struct msm_gem_submit { 
		struct drm_msm_gem_submit_reloc *relocs;

	} *cmd;  /* array of size nr_cmds */

	struct {

/* make sure these don't conflict w/ MSM_SUBMIT_BO_x */

#define BO_VALID    0x8000   /* is current addr in cmdstream correct/valid? */

#define BO_LOCKED   0x4000   /* obj lock is held */

#define BO_ACTIVE   0x2000   /* active refcnt is held */

#define BO_PINNED   0x1000   /* obj is pinned and on active list */

		uint32_t flags;

		union {

			struct msm_gem_object *obj;

drivers/gpu/drm/msm/msm_gem_submit.c

+7 −8
Original line number Diff line number Diff line
@@ -21,12 +21,6 @@ 
 * Cmdstream submission:

 */



/* make sure these don't conflict w/ MSM_SUBMIT_BO_x */

#define BO_VALID    0x8000   /* is current addr in cmdstream correct/valid? */

#define BO_LOCKED   0x4000   /* obj lock is held */

#define BO_ACTIVE   0x2000   /* active refcnt is held */

#define BO_PINNED   0x1000   /* obj is pinned and on active list */



static struct msm_gem_submit *submit_create(struct drm_device *dev,

		struct msm_gpu *gpu,

		struct msm_gpu_submitqueue *queue, uint32_t nr_bos,
@@ -231,6 +225,13 @@ static void submit_cleanup_bo(struct msm_gem_submit *submit, int i, 
	struct drm_gem_object *obj = &submit->bos[i].obj->base;

	unsigned flags = submit->bos[i].flags & cleanup_flags;



	/*

	 * Clear flags bit before dropping lock, so that the msm_job_run()

	 * path isn't racing with submit_cleanup() (ie. the read/modify/

	 * write is protected by the obj lock in all paths)

	 */

	submit->bos[i].flags &= ~cleanup_flags;



	if (flags & BO_PINNED)

		msm_gem_unpin_vma_locked(obj, submit->bos[i].vma);
@@ -239,8 +240,6 @@ static void submit_cleanup_bo(struct msm_gem_submit *submit, int i, 


	if (flags & BO_LOCKED)

		dma_resv_unlock(obj->resv);



	submit->bos[i].flags &= ~cleanup_flags;

}



static void submit_unlock_unpin_bo(struct msm_gem_submit *submit, int i)