[PATCH 1/7] drm/ttm: protect against reentrant bind in the drivers
Dave Airlie
airlied at gmail.com
Thu Sep 17 04:30:34 UTC 2020
From: Dave Airlie <airlied at redhat.com>
This moves the generic tracking into the drivers and protects
against reentrancy in the drivers. It fixes up radeon and agp
to be able to query the bound status as that is required.
Signed-off-by: Dave Airlie <airlied at redhat.com>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 12 +++++++++
drivers/gpu/drm/nouveau/nouveau_bo.c | 5 +++-
drivers/gpu/drm/nouveau/nouveau_sgdma.c | 8 +++++-
drivers/gpu/drm/radeon/radeon.h | 1 +
drivers/gpu/drm/radeon/radeon_mn.c | 2 +-
drivers/gpu/drm/radeon/radeon_ttm.c | 31 ++++++++++++++++++++++
drivers/gpu/drm/ttm/ttm_agp_backend.c | 14 ++++++++++
drivers/gpu/drm/ttm/ttm_bo.c | 20 ++------------
drivers/gpu/drm/ttm/ttm_bo_util.c | 5 ----
drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c | 22 ++++++++++++---
include/drm/ttm/ttm_bo_api.h | 1 -
include/drm/ttm/ttm_bo_driver.h | 14 ----------
include/drm/ttm/ttm_tt.h | 1 +
13 files changed, 91 insertions(+), 45 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index e86f8f6371c4..2851cf30091a 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -813,6 +813,7 @@ struct amdgpu_ttm_tt {
uint64_t userptr;
struct task_struct *usertask;
uint32_t userflags;
+ bool bound;
#if IS_ENABLED(CONFIG_DRM_AMDGPU_USERPTR)
struct hmm_range *range;
#endif
@@ -1110,6 +1111,12 @@ static int amdgpu_ttm_backend_bind(struct ttm_bo_device *bdev,
uint64_t flags;
int r = 0;
+ if (!bo_mem)
+ return -EINVAL;
+
+ if (gtt->bound)
+ return 0;
+
if (gtt->userptr) {
r = amdgpu_ttm_tt_pin_userptr(bdev, ttm);
if (r) {
@@ -1143,6 +1150,7 @@ static int amdgpu_ttm_backend_bind(struct ttm_bo_device *bdev,
if (r)
DRM_ERROR("failed to bind %lu pages at 0x%08llX\n",
ttm->num_pages, gtt->offset);
+ gtt->bound = true;
return r;
}
@@ -1236,6 +1244,9 @@ static void amdgpu_ttm_backend_unbind(struct ttm_bo_device *bdev,
struct amdgpu_ttm_tt *gtt = (void *)ttm;
int r;
+ if (!gtt->bound)
+ return;
+
/* if the pages have userptr pinning then clear that first */
if (gtt->userptr)
amdgpu_ttm_tt_unpin_userptr(bdev, ttm);
@@ -1248,6 +1259,7 @@ static void amdgpu_ttm_backend_unbind(struct ttm_bo_device *bdev,
if (r)
DRM_ERROR("failed to unbind %lu pages at 0x%08llX\n",
gtt->ttm.ttm.num_pages, gtt->offset);
+ gtt->bound = false;
}
static void amdgpu_ttm_backend_destroy(struct ttm_bo_device *bdev,
diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c b/drivers/gpu/drm/nouveau/nouveau_bo.c
index aea201d9c513..616d8844ba97 100644
--- a/drivers/gpu/drm/nouveau/nouveau_bo.c
+++ b/drivers/gpu/drm/nouveau/nouveau_bo.c
@@ -718,7 +718,10 @@ nouveau_ttm_tt_bind(struct ttm_bo_device *bdev, struct ttm_tt *ttm,
{
#if IS_ENABLED(CONFIG_AGP)
struct nouveau_drm *drm = nouveau_bdev(bdev);
-
+#endif
+ if (!reg)
+ return -EINVAL;
+#if IS_ENABLED(CONFIG_AGP)
if (drm->agp.bridge)
return ttm_agp_bind(ttm, reg);
#endif
diff --git a/drivers/gpu/drm/nouveau/nouveau_sgdma.c b/drivers/gpu/drm/nouveau/nouveau_sgdma.c
index 05e542254e1f..21fb92770ea2 100644
--- a/drivers/gpu/drm/nouveau/nouveau_sgdma.c
+++ b/drivers/gpu/drm/nouveau/nouveau_sgdma.c
@@ -33,6 +33,9 @@ nouveau_sgdma_bind(struct ttm_bo_device *bdev, struct ttm_tt *ttm, struct ttm_re
struct nouveau_mem *mem = nouveau_mem(reg);
int ret;
+ if (nvbe->mem)
+ return 0;
+
ret = nouveau_mem_host(reg, &nvbe->ttm);
if (ret)
return ret;
@@ -53,7 +56,10 @@ void
nouveau_sgdma_unbind(struct ttm_bo_device *bdev, struct ttm_tt *ttm)
{
struct nouveau_sgdma_be *nvbe = (struct nouveau_sgdma_be *)ttm;
- nouveau_mem_fini(nvbe->mem);
+ if (nvbe->mem) {
+ nouveau_mem_fini(nvbe->mem);
+ nvbe->mem = NULL;
+ }
}
struct ttm_tt *
diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h
index df6f0b49836b..a6d8de01194a 100644
--- a/drivers/gpu/drm/radeon/radeon.h
+++ b/drivers/gpu/drm/radeon/radeon.h
@@ -2820,6 +2820,7 @@ extern int radeon_ttm_tt_set_userptr(struct radeon_device *rdev,
uint32_t flags);
extern bool radeon_ttm_tt_has_userptr(struct radeon_device *rdev, struct ttm_tt *ttm);
extern bool radeon_ttm_tt_is_readonly(struct radeon_device *rdev, struct ttm_tt *ttm);
+bool radeon_ttm_tt_is_bound(struct ttm_bo_device *bdev, struct ttm_tt *ttm);
extern void radeon_vram_location(struct radeon_device *rdev, struct radeon_mc *mc, u64 base);
extern void radeon_gtt_location(struct radeon_device *rdev, struct radeon_mc *mc);
extern int radeon_resume_kms(struct drm_device *dev, bool resume, bool fbcon);
diff --git a/drivers/gpu/drm/radeon/radeon_mn.c b/drivers/gpu/drm/radeon/radeon_mn.c
index eb46d2220236..97b9b6dd6dd3 100644
--- a/drivers/gpu/drm/radeon/radeon_mn.c
+++ b/drivers/gpu/drm/radeon/radeon_mn.c
@@ -53,7 +53,7 @@ static bool radeon_mn_invalidate(struct mmu_interval_notifier *mn,
struct ttm_operation_ctx ctx = { false, false };
long r;
- if (!bo->tbo.ttm || !ttm_bo_tt_is_bound(&bo->tbo))
+ if (!bo->tbo.ttm || !radeon_ttm_tt_is_bound(bo->tbo.bdev, bo->tbo.ttm))
return true;
if (!mmu_notifier_range_blockable(range))
diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c
index c6c1008fefd2..fc8bbca28b34 100644
--- a/drivers/gpu/drm/radeon/radeon_ttm.c
+++ b/drivers/gpu/drm/radeon/radeon_ttm.c
@@ -420,6 +420,7 @@ struct radeon_ttm_tt {
uint64_t userptr;
struct mm_struct *usermm;
uint32_t userflags;
+ bool bound;
};
/* prepare the sg table with the user pages */
@@ -513,6 +514,13 @@ static void radeon_ttm_tt_unpin_userptr(struct ttm_bo_device *bdev, struct ttm_t
sg_free_table(ttm->sg);
}
+static bool radeon_ttm_backend_is_bound(struct ttm_tt *ttm)
+{
+ struct radeon_ttm_tt *gtt = (void*)ttm;
+
+ return (gtt->bound);
+}
+
static int radeon_ttm_backend_bind(struct ttm_bo_device *bdev,
struct ttm_tt *ttm,
struct ttm_resource *bo_mem)
@@ -523,6 +531,9 @@ static int radeon_ttm_backend_bind(struct ttm_bo_device *bdev,
RADEON_GART_PAGE_WRITE;
int r;
+ if (gtt->bound)
+ return 0;
+
if (gtt->userptr) {
radeon_ttm_tt_pin_userptr(bdev, ttm);
flags &= ~RADEON_GART_PAGE_WRITE;
@@ -542,6 +553,7 @@ static int radeon_ttm_backend_bind(struct ttm_bo_device *bdev,
ttm->num_pages, (unsigned)gtt->offset);
return r;
}
+ gtt->bound = true;
return 0;
}
@@ -550,10 +562,14 @@ static void radeon_ttm_backend_unbind(struct ttm_bo_device *bdev, struct ttm_tt
struct radeon_ttm_tt *gtt = (void *)ttm;
struct radeon_device *rdev = radeon_get_rdev(bdev);
+ if (!gtt->bound)
+ return;
+
radeon_gart_unbind(rdev, gtt->offset, ttm->num_pages);
if (gtt->userptr)
radeon_ttm_tt_unpin_userptr(bdev, ttm);
+ gtt->bound = false;
}
static void radeon_ttm_backend_destroy(struct ttm_bo_device *bdev, struct ttm_tt *ttm)
@@ -689,12 +705,27 @@ int radeon_ttm_tt_set_userptr(struct radeon_device *rdev,
return 0;
}
+bool radeon_ttm_tt_is_bound(struct ttm_bo_device *bdev,
+ struct ttm_tt *ttm)
+{
+#if IS_ENABLED(CONFIG_AGP)
+ struct radeon_device *rdev = radeon_get_rdev(bdev);
+ if (rdev->flags & RADEON_IS_AGP)
+ return ttm_agp_is_bound(ttm);
+#endif
+ return radeon_ttm_backend_is_bound(ttm);
+}
+
static int radeon_ttm_tt_bind(struct ttm_bo_device *bdev,
struct ttm_tt *ttm,
struct ttm_resource *bo_mem)
{
+#if IS_ENABLED(CONFIG_AGP)
struct radeon_device *rdev = radeon_get_rdev(bdev);
+#endif
+ if (!bo_mem)
+ return -EINVAL;
#if IS_ENABLED(CONFIG_AGP)
if (rdev->flags & RADEON_IS_AGP)
return ttm_agp_bind(ttm, bo_mem);
diff --git a/drivers/gpu/drm/ttm/ttm_agp_backend.c b/drivers/gpu/drm/ttm/ttm_agp_backend.c
index 7b36fdaab766..a98fd795b752 100644
--- a/drivers/gpu/drm/ttm/ttm_agp_backend.c
+++ b/drivers/gpu/drm/ttm/ttm_agp_backend.c
@@ -57,6 +57,9 @@ int ttm_agp_bind(struct ttm_tt *ttm, struct ttm_resource *bo_mem)
int ret, cached = (bo_mem->placement & TTM_PL_FLAG_CACHED);
unsigned i;
+ if (agp_be->mem)
+ return 0;
+
mem = agp_allocate_memory(agp_be->bridge, ttm->num_pages, AGP_USER_MEMORY);
if (unlikely(mem == NULL))
return -ENOMEM;
@@ -98,6 +101,17 @@ void ttm_agp_unbind(struct ttm_tt *ttm)
}
EXPORT_SYMBOL(ttm_agp_unbind);
+bool ttm_agp_is_bound(struct ttm_tt *ttm)
+{
+ struct ttm_agp_backend *agp_be = container_of(ttm, struct ttm_agp_backend, ttm);
+
+ if (!ttm)
+ return false;
+
+ return (agp_be->mem != NULL);
+}
+EXPORT_SYMBOL(ttm_agp_is_bound);
+
void ttm_agp_destroy(struct ttm_tt *ttm)
{
struct ttm_agp_backend *agp_be = container_of(ttm, struct ttm_agp_backend, ttm);
diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index 17010e7d0ea9..649a7d0a0766 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -1627,27 +1627,11 @@ void ttm_bo_tt_destroy(struct ttm_buffer_object *bo)
int ttm_bo_tt_bind(struct ttm_buffer_object *bo, struct ttm_resource *mem)
{
- int ret;
-
- if (!bo->ttm)
- return -EINVAL;
-
- if (ttm_bo_tt_is_bound(bo))
- return 0;
-
- ret = bo->bdev->driver->ttm_tt_bind(bo->bdev, bo->ttm, mem);
- if (unlikely(ret != 0))
- return ret;
-
- ttm_bo_tt_set_bound(bo);
- return 0;
+ return bo->bdev->driver->ttm_tt_bind(bo->bdev, bo->ttm, mem);
}
EXPORT_SYMBOL(ttm_bo_tt_bind);
void ttm_bo_tt_unbind(struct ttm_buffer_object *bo)
{
- if (ttm_bo_tt_is_bound(bo)) {
- bo->bdev->driver->ttm_tt_unbind(bo->bdev, bo->ttm);
- ttm_bo_tt_set_unbound(bo);
- }
+ bo->bdev->driver->ttm_tt_unbind(bo->bdev, bo->ttm);
}
diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c b/drivers/gpu/drm/ttm/ttm_bo_util.c
index 980368049d68..919489f6a5a3 100644
--- a/drivers/gpu/drm/ttm/ttm_bo_util.c
+++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
@@ -574,10 +574,8 @@ int ttm_bo_move_accel_cleanup(struct ttm_buffer_object *bo,
if (man->use_tt) {
ghost_obj->ttm = NULL;
- ttm_bo_tt_set_unbound(ghost_obj);
} else {
bo->ttm = NULL;
- ttm_bo_tt_set_unbound(bo);
}
dma_resv_unlock(&ghost_obj->base._resv);
@@ -633,10 +631,8 @@ int ttm_bo_pipeline_move(struct ttm_buffer_object *bo,
if (to->use_tt) {
ghost_obj->ttm = NULL;
- ttm_bo_tt_set_unbound(ghost_obj);
} else {
bo->ttm = NULL;
- ttm_bo_tt_set_unbound(bo);
}
dma_resv_unlock(&ghost_obj->base._resv);
@@ -701,7 +697,6 @@ int ttm_bo_pipeline_gutting(struct ttm_buffer_object *bo)
memset(&bo->mem, 0, sizeof(bo->mem));
bo->mem.mem_type = TTM_PL_SYSTEM;
bo->ttm = NULL;
- ttm_bo_tt_set_unbound(bo);
dma_resv_unlock(&ghost->base._resv);
ttm_bo_put(ghost);
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
index 3458c5c3531d..01146b27c9a1 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
@@ -267,6 +267,7 @@ struct vmw_ttm_tt {
struct vmw_sg_table vsgt;
uint64_t sg_alloc_size;
bool mapped;
+ bool bound;
};
const size_t vmw_tt_size = sizeof(struct vmw_ttm_tt);
@@ -565,7 +566,13 @@ static int vmw_ttm_bind(struct ttm_bo_device *bdev,
{
struct vmw_ttm_tt *vmw_be =
container_of(ttm, struct vmw_ttm_tt, dma_ttm.ttm);
- int ret;
+ int ret = 0;
+
+ if (!bo_mem)
+ return -EINVAL;
+
+ if (vmw_be->bound)
+ return 0;
ret = vmw_ttm_map_dma(vmw_be);
if (unlikely(ret != 0))
@@ -576,8 +583,9 @@ static int vmw_ttm_bind(struct ttm_bo_device *bdev,
switch (bo_mem->mem_type) {
case VMW_PL_GMR:
- return vmw_gmr_bind(vmw_be->dev_priv, &vmw_be->vsgt,
+ ret = vmw_gmr_bind(vmw_be->dev_priv, &vmw_be->vsgt,
ttm->num_pages, vmw_be->gmr_id);
+ break;
case VMW_PL_MOB:
if (unlikely(vmw_be->mob == NULL)) {
vmw_be->mob =
@@ -586,13 +594,15 @@ static int vmw_ttm_bind(struct ttm_bo_device *bdev,
return -ENOMEM;
}
- return vmw_mob_bind(vmw_be->dev_priv, vmw_be->mob,
+ ret = vmw_mob_bind(vmw_be->dev_priv, vmw_be->mob,
&vmw_be->vsgt, ttm->num_pages,
vmw_be->gmr_id);
+ break;
default:
BUG();
}
- return 0;
+ vmw_be->bound = true;
+ return ret;
}
static void vmw_ttm_unbind(struct ttm_bo_device *bdev,
@@ -601,6 +611,9 @@ static void vmw_ttm_unbind(struct ttm_bo_device *bdev,
struct vmw_ttm_tt *vmw_be =
container_of(ttm, struct vmw_ttm_tt, dma_ttm.ttm);
+ if (!vmw_be->bound)
+ return;
+
switch (vmw_be->mem_type) {
case VMW_PL_GMR:
vmw_gmr_unbind(vmw_be->dev_priv, vmw_be->gmr_id);
@@ -614,6 +627,7 @@ static void vmw_ttm_unbind(struct ttm_bo_device *bdev,
if (vmw_be->dev_priv->map_mode == vmw_dma_map_bind)
vmw_ttm_unmap_dma(vmw_be);
+ vmw_be->bound = false;
}
diff --git a/include/drm/ttm/ttm_bo_api.h b/include/drm/ttm/ttm_bo_api.h
index 1d20a7f15a7a..36ff64e2736c 100644
--- a/include/drm/ttm/ttm_bo_api.h
+++ b/include/drm/ttm/ttm_bo_api.h
@@ -141,7 +141,6 @@ struct ttm_buffer_object {
struct ttm_resource mem;
struct file *persistent_swap_storage;
struct ttm_tt *ttm;
- bool ttm_bound;
bool evicted;
bool deleted;
diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h
index e66672f703a3..7846dfa507f7 100644
--- a/include/drm/ttm/ttm_bo_driver.h
+++ b/include/drm/ttm/ttm_bo_driver.h
@@ -698,20 +698,6 @@ int ttm_bo_tt_bind(struct ttm_buffer_object *bo, struct ttm_resource *mem);
*/
void ttm_bo_tt_unbind(struct ttm_buffer_object *bo);
-static inline bool ttm_bo_tt_is_bound(struct ttm_buffer_object *bo)
-{
- return bo->ttm_bound;
-}
-
-static inline void ttm_bo_tt_set_unbound(struct ttm_buffer_object *bo)
-{
- bo->ttm_bound = false;
-}
-
-static inline void ttm_bo_tt_set_bound(struct ttm_buffer_object *bo)
-{
- bo->ttm_bound = true;
-}
/**
* ttm_bo_tt_destroy.
*/
diff --git a/include/drm/ttm/ttm_tt.h b/include/drm/ttm/ttm_tt.h
index c777b72063db..4e906e32d08c 100644
--- a/include/drm/ttm/ttm_tt.h
+++ b/include/drm/ttm/ttm_tt.h
@@ -219,6 +219,7 @@ struct ttm_tt *ttm_agp_tt_create(struct ttm_buffer_object *bo,
int ttm_agp_bind(struct ttm_tt *ttm, struct ttm_resource *bo_mem);
void ttm_agp_unbind(struct ttm_tt *ttm);
void ttm_agp_destroy(struct ttm_tt *ttm);
+bool ttm_agp_is_bound(struct ttm_tt *ttm);
#endif
#endif
--
2.27.0
More information about the dri-devel
mailing list