Attachment 65101 Details for Bug 45018 – Properly protect virtual address kernel 3.5 v2

[patch] Properly protect virtual address kernel 3.5 v2

0001-drm-radeon-fence-virtual-address-and-free-it-once-id.patch (text/plain), 6.33 KB, created by Jerome Glisse on 2012-08-03 19:04:54 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Jerome Glisse

Created: 2012-08-03 19:04:54 UTC

Size: 6.33 KB

patch

obsolete

>From bf3f1cbe224f6e79928cd7bd1e1b8853ac5c7443 Mon Sep 17 00:00:00 2001
>From: Jerome Glisse <jglisse@redhat.com>
>Date: Fri, 3 Aug 2012 12:45:15 -0400
>Subject: [PATCH] drm/radeon: fence virtual address and free it once idle
> [3.5] v2
>
>Virtual address need to be fenced to know when we can safely remove it.
>This patch also properly clear the pagetable. Previously it was
>serouisly broken.
>
>v2: For to update pagetable when unbinding bo (don't bailout if
>    bo_va->valid is true).
>
>Signed-off-by: Jerome Glisse <jglisse@redhat.com>
>---
> drivers/gpu/drm/radeon/radeon.h        |  1 +
> drivers/gpu/drm/radeon/radeon_cs.c     | 32 +++++++++++++++++++++++++++++---
> drivers/gpu/drm/radeon/radeon_gart.c   | 24 ++++++++++++++++++++++--
> drivers/gpu/drm/radeon/radeon_gem.c    | 13 ++-----------
> drivers/gpu/drm/radeon/radeon_object.c |  6 +-----
> 5 files changed, 55 insertions(+), 21 deletions(-)
>
>diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h
>index fefcca5..01d2a87 100644
>--- a/drivers/gpu/drm/radeon/radeon.h
>+++ b/drivers/gpu/drm/radeon/radeon.h
>@@ -323,6 +323,7 @@ struct radeon_bo_va {
> 	uint64_t			soffset;
> 	uint64_t			eoffset;
> 	uint32_t			flags;
>+	struct radeon_fence		*fence;
> 	bool				valid;
> };
> 
>diff --git a/drivers/gpu/drm/radeon/radeon_cs.c b/drivers/gpu/drm/radeon/radeon_cs.c
>index 142f894..70f6d08 100644
>--- a/drivers/gpu/drm/radeon/radeon_cs.c
>+++ b/drivers/gpu/drm/radeon/radeon_cs.c
>@@ -294,6 +294,30 @@ int radeon_cs_parser_init(struct radeon_cs_parser *p, void *data)
> 	return 0;
> }
> 
>+static void radeon_bo_vm_fence_va(struct radeon_cs_parser *parser,
>+				  struct radeon_fence *fence)
>+{
>+	struct radeon_fpriv *fpriv = parser->filp->driver_priv;
>+	struct radeon_vm *vm = &fpriv->vm;
>+	struct radeon_bo_list *lobj;
>+	int r;
>+
>+	if (parser->chunk_ib_idx == -1)
>+		return;
>+	if ((parser->cs_flags & RADEON_CS_USE_VM) == 0)
>+		return;
>+
>+	list_for_each_entry(lobj, &parser->validated, tv.head) {
>+		struct radeon_bo_va *bo_va;
>+		struct radeon_bo *rbo = lobj->bo;
>+
>+		bo_va = radeon_bo_va(rbo, vm);
>+		radeon_fence_unref(&bo_va->fence);
>+		bo_va->fence = radeon_fence_ref(fence);
>+	}
>+	return 0;
>+}
>+
> /**
>  * cs_parser_fini() - clean parser states
>  * @parser:	parser structure holding parsing context.
>@@ -306,11 +330,14 @@ static void radeon_cs_parser_fini(struct radeon_cs_parser *parser, int error)
> {
> 	unsigned i;
> 
>-	if (!error)
>+	if (!error) {
>+		/* fence all bo va before ttm_eu_fence_buffer_objects so bo are still reserved */
>+		radeon_bo_vm_fence_va(parser, parser->ib.fence);
> 		ttm_eu_fence_buffer_objects(&parser->validated,
> 					    parser->ib.fence);
>-	else
>+	} else {
> 		ttm_eu_backoff_reservation(&parser->validated);
>+	}
> 
> 	if (parser->relocs != NULL) {
> 		for (i = 0; i < parser->nrelocs; i++) {
>@@ -407,7 +434,6 @@ static int radeon_cs_ib_vm_chunk(struct radeon_device *rdev,
> 
> 	if (parser->chunk_ib_idx == -1)
> 		return 0;
>-
> 	if ((parser->cs_flags & RADEON_CS_USE_VM) == 0)
> 		return 0;
> 
>diff --git a/drivers/gpu/drm/radeon/radeon_gart.c b/drivers/gpu/drm/radeon/radeon_gart.c
>index 84b648a..f651f22 100644
>--- a/drivers/gpu/drm/radeon/radeon_gart.c
>+++ b/drivers/gpu/drm/radeon/radeon_gart.c
>@@ -564,7 +564,7 @@ int radeon_vm_bo_update_pte(struct radeon_device *rdev,
> 		return -EINVAL;
> 	}
> 
>-	if (bo_va->valid)
>+	if (bo_va->valid && mem)
> 		return 0;
> 
> 	ngpu_pages = radeon_bo_ngpu_pages(bo);
>@@ -597,11 +597,27 @@ int radeon_vm_bo_rmv(struct radeon_device *rdev,
> 		     struct radeon_bo *bo)
> {
> 	struct radeon_bo_va *bo_va;
>+	int r;
> 
> 	bo_va = radeon_bo_va(bo, vm);
> 	if (bo_va == NULL)
> 		return 0;
> 
>+	/* wait for va use to end */
>+	while (bo_va->fence) {
>+		r = radeon_fence_wait(bo_va->fence, false);
>+		if (r) {
>+			DRM_ERROR("error while waiting for fence: %d\n", r);
>+		}
>+		if (r == -EDEADLK) {
>+			r = radeon_gpu_reset(rdev);
>+			if (!r)
>+				continue;
>+		}
>+		break;
>+	}
>+	radeon_fence_unref(&bo_va->fence);
>+
> 	radeon_mutex_lock(&rdev->cs_mutex);
> 	mutex_lock(&vm->mutex);
> 	radeon_vm_bo_update_pte(rdev, vm, bo, NULL);
>@@ -661,12 +677,15 @@ void radeon_vm_fini(struct radeon_device *rdev, struct radeon_vm *vm)
> 	radeon_vm_unbind_locked(rdev, vm);
> 	radeon_mutex_unlock(&rdev->cs_mutex);
> 
>-	/* remove all bo */
>+	/* remove all bo at this point non are busy any more because unbind
>+	 * waited for the last vm fence to signal
>+	 */
> 	r = radeon_bo_reserve(rdev->ring_tmp_bo.bo, false);
> 	if (!r) {
> 		bo_va = radeon_bo_va(rdev->ring_tmp_bo.bo, vm);
> 		list_del_init(&bo_va->bo_list);
> 		list_del_init(&bo_va->vm_list);
>+		radeon_fence_unref(&bo_va->fence);
> 		radeon_bo_unreserve(rdev->ring_tmp_bo.bo);
> 		kfree(bo_va);
> 	}
>@@ -678,6 +697,7 @@ void radeon_vm_fini(struct radeon_device *rdev, struct radeon_vm *vm)
> 		r = radeon_bo_reserve(bo_va->bo, false);
> 		if (!r) {
> 			list_del_init(&bo_va->bo_list);
>+			radeon_fence_unref(&bo_va->fence);
> 			radeon_bo_unreserve(bo_va->bo);
> 			kfree(bo_va);
> 		}
>diff --git a/drivers/gpu/drm/radeon/radeon_gem.c b/drivers/gpu/drm/radeon/radeon_gem.c
>index 21ec9f5..12207d9 100644
>--- a/drivers/gpu/drm/radeon/radeon_gem.c
>+++ b/drivers/gpu/drm/radeon/radeon_gem.c
>@@ -134,25 +134,16 @@ void radeon_gem_object_close(struct drm_gem_object *obj,
> 	struct radeon_device *rdev = rbo->rdev;
> 	struct radeon_fpriv *fpriv = file_priv->driver_priv;
> 	struct radeon_vm *vm = &fpriv->vm;
>-	struct radeon_bo_va *bo_va, *tmp;
> 
> 	if (rdev->family < CHIP_CAYMAN) {
> 		return;
> 	}
> 
> 	if (radeon_bo_reserve(rbo, false)) {
>+		dev_err(rdev->dev, "leaking bo va because we fail to reserve bo\n");
> 		return;
> 	}
>-	list_for_each_entry_safe(bo_va, tmp, &rbo->va, bo_list) {
>-		if (bo_va->vm == vm) {
>-			/* remove from this vm address space */
>-			mutex_lock(&vm->mutex);
>-			list_del(&bo_va->vm_list);
>-			mutex_unlock(&vm->mutex);
>-			list_del(&bo_va->bo_list);
>-			kfree(bo_va);
>-		}
>-	}
>+	radeon_vm_bo_rmv(rdev, vm, rbo);
> 	radeon_bo_unreserve(rbo);
> }
> 
>diff --git a/drivers/gpu/drm/radeon/radeon_object.c b/drivers/gpu/drm/radeon/radeon_object.c
>index 830f1a7..1b2289f 100644
>--- a/drivers/gpu/drm/radeon/radeon_object.c
>+++ b/drivers/gpu/drm/radeon/radeon_object.c
>@@ -52,11 +52,7 @@ void radeon_bo_clear_va(struct radeon_bo *bo)
> 
> 	list_for_each_entry_safe(bo_va, tmp, &bo->va, bo_list) {
> 		/* remove from all vm address space */
>-		mutex_lock(&bo_va->vm->mutex);
>-		list_del(&bo_va->vm_list);
>-		mutex_unlock(&bo_va->vm->mutex);
>-		list_del(&bo_va->bo_list);
>-		kfree(bo_va);
>+		radeon_vm_bo_rmv(bo->rdev, bo_va->vm, bo);
> 	}
> }
> 
>-- 
>1.7.11.2
>

Actions: View | Splinter Review

Attachments on bug 45018: 55888 | 55889 | 55912 | 55913 | 57178 | 57234 | 57235 | 57237 | 57645 | 57740 | 58846 | 60582 | 62618 | 62619 | 64052 | 64053 | 64585 | 65013 | 65014 | 65051 | 65093 | 65095 | 65096 | 65097 | 65098 | 65101 | 65102

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.