There was a possible race in vmw_update_seqno. Because of this race it
was possible for last_read_seqno to go backwards. Remove this function
and replace it with vmw_update_fences which now sets and returns the
last_read_seqno while holding the fence lock. This serialization via the
fence lock ensures that last_read_seqno is monotonic again.
Signed-off-by: Ian Forbes <ian.for...@broadcom.com>
---
drivers/gpu/drm/vmwgfx/vmwgfx_cmd.c | 2 +-
drivers/gpu/drm/vmwgfx/vmwgfx_drv.h | 1 -
drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c | 3 +--
drivers/gpu/drm/vmwgfx/vmwgfx_fence.c | 18 +++++++++---------
drivers/gpu/drm/vmwgfx/vmwgfx_fence.h | 2 +-
drivers/gpu/drm/vmwgfx/vmwgfx_irq.c | 12 +-----------
6 files changed, 13 insertions(+), 25 deletions(-)
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_cmd.c
b/drivers/gpu/drm/vmwgfx/vmwgfx_cmd.c
index dd4ca6a9c690..8fe02131a6c4 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_cmd.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_cmd.c
@@ -544,7 +544,7 @@ int vmw_cmd_send_fence(struct vmw_private *dev_priv,
uint32_t *seqno)
cmd_fence = (struct svga_fifo_cmd_fence *) fm;
cmd_fence->fence = *seqno;
vmw_cmd_commit_flush(dev_priv, bytes);
- vmw_update_seqno(dev_priv);
+ vmw_fences_update(dev_priv->fman);
out_err:
return ret;
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
index 594af8eb04c6..6d4a68f0ae37 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
@@ -1006,7 +1006,6 @@ extern int vmw_fallback_wait(struct vmw_private *dev_priv,
uint32_t seqno,
bool interruptible,
unsigned long timeout);
-extern void vmw_update_seqno(struct vmw_private *dev_priv);
extern void vmw_seqno_waiter_add(struct vmw_private *dev_priv);
extern void vmw_seqno_waiter_remove(struct vmw_private *dev_priv);
extern void vmw_goal_waiter_add(struct vmw_private *dev_priv);
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c
b/drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c
index e831e324e737..90ce5372343b 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c
@@ -3878,8 +3878,7 @@ vmw_execbuf_copy_fence_user(struct vmw_private *dev_priv,
fence_rep.handle = fence_handle;
fence_rep.seqno = fence->base.seqno;
- vmw_update_seqno(dev_priv);
- fence_rep.passed_seqno = dev_priv->last_read_seqno;
+ fence_rep.passed_seqno = vmw_fences_update(dev_priv->fman);
}
/*
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_fence.c
b/drivers/gpu/drm/vmwgfx/vmwgfx_fence.c
index 588d50ababf6..9d1465558839 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_fence.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_fence.c
@@ -172,7 +172,7 @@ vmwgfx_wait_cb(struct dma_fence *fence, struct dma_fence_cb
*cb)
wake_up_process(wait->task);
}
-static void __vmw_fences_update(struct vmw_fence_manager *fman);
+static u32 __vmw_fences_update(struct vmw_fence_manager *fman);
static long vmw_fence_wait(struct dma_fence *f, bool intr, signed long timeout)
{
@@ -457,7 +457,7 @@ static bool vmw_fence_goal_check_locked(struct
vmw_fence_obj *fence)
return true;
}
-static void __vmw_fences_update(struct vmw_fence_manager *fman)
+static u32 __vmw_fences_update(struct vmw_fence_manager *fman)
{
struct vmw_fence_obj *fence, *next_fence;
struct list_head action_list;
@@ -495,13 +495,16 @@ static void __vmw_fences_update(struct vmw_fence_manager
*fman)
if (!list_empty(&fman->cleanup_list))
(void) schedule_work(&fman->work);
+ return (fman->dev_priv->last_read_seqno = seqno);
}
-void vmw_fences_update(struct vmw_fence_manager *fman)
+u32 vmw_fences_update(struct vmw_fence_manager *fman)
{
+ u32 seqno;
spin_lock(&fman->lock);
- __vmw_fences_update(fman);
+ seqno = __vmw_fences_update(fman);
spin_unlock(&fman->lock);
+ return seqno;
}
bool vmw_fence_obj_signaled(struct vmw_fence_obj *fence)
@@ -778,7 +781,6 @@ int vmw_fence_obj_signaled_ioctl(struct drm_device *dev,
void *data,
(struct drm_vmw_fence_signaled_arg *) data;
struct ttm_base_object *base;
struct vmw_fence_obj *fence;
- struct vmw_fence_manager *fman;
struct ttm_object_file *tfile = vmw_fpriv(file_priv)->tfile;
struct vmw_private *dev_priv = vmw_priv(dev);
@@ -787,14 +789,12 @@ int vmw_fence_obj_signaled_ioctl(struct drm_device *dev,
void *data,
return PTR_ERR(base);
fence = &(container_of(base, struct vmw_user_fence, base)->fence);
- fman = fman_from_fence(fence);
arg->signaled = vmw_fence_obj_signaled(fence);
arg->signaled_flags = arg->flags;
- spin_lock(&fman->lock);
- arg->passed_seqno = dev_priv->last_read_seqno;
- spin_unlock(&fman->lock);
+ arg->passed_seqno = READ_ONCE(dev_priv->last_read_seqno);
+
ttm_base_object_unref(&base);
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_fence.h
b/drivers/gpu/drm/vmwgfx/vmwgfx_fence.h
index a7eee579c76a..10264dab5f6a 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_fence.h
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_fence.h
@@ -86,7 +86,7 @@ vmw_fence_obj_reference(struct vmw_fence_obj *fence)
return fence;
}
-extern void vmw_fences_update(struct vmw_fence_manager *fman);
+u32 vmw_fences_update(struct vmw_fence_manager *fman);
extern bool vmw_fence_obj_signaled(struct vmw_fence_obj *fence);
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_irq.c
b/drivers/gpu/drm/vmwgfx/vmwgfx_irq.c
index 086e69a130d4..548ef2f86508 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_irq.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_irq.c
@@ -123,16 +123,6 @@ static bool vmw_fifo_idle(struct vmw_private *dev_priv,
uint32_t seqno)
return (vmw_read(dev_priv, SVGA_REG_BUSY) == 0);
}
-void vmw_update_seqno(struct vmw_private *dev_priv)
-{
- uint32_t seqno = vmw_fence_read(dev_priv);
-
- if (dev_priv->last_read_seqno != seqno) {
- dev_priv->last_read_seqno = seqno;
- vmw_fences_update(dev_priv->fman);
- }
-}
-
bool vmw_seqno_passed(struct vmw_private *dev_priv,
uint32_t seqno)
{
@@ -141,7 +131,7 @@ bool vmw_seqno_passed(struct vmw_private *dev_priv,
if (likely(dev_priv->last_read_seqno - seqno < VMW_FENCE_WRAP))
return true;
- vmw_update_seqno(dev_priv);
+ vmw_fences_update(dev_priv->fman);
if (likely(dev_priv->last_read_seqno - seqno < VMW_FENCE_WRAP))
return true;
--
2.49.0
