[PATCH 1/4] drm/amdkfd: don't drop an SVM CWSR reference that was never taken

[Jesse Zhang]

When kfd_queue_acquire_buffers() fails, its error path calls
kfd_queue_release_buffers(), which also drops a queue_refcount on the
SVM range at ctx_save_restore_area_address. The SVM range is the last
buffer acquired, and kfd_queue_buffer_svm_get() takes no reference when
it fails, so on every error path this reference was never taken. If
another queue of the process uses that CWSR range, its reference is
dropped instead, and unmapping the range no longer evicts that queue
while its CWSR area is unmapped from the GPU.

On navi31, with queue A using an SVM CWSR range: munmap() of the range
logs

  Freeing queue vital buffer 0x..., queue evicted

but after a CREATE_QUEUE that reuses A's CWSR address with an unmapped
write pointer (-EINVAL), the same munmap() leaves A running without the
message.

Only put the BOs on the acquire error path. The warning and the eviction
are back after the failed CREATE_QUEUE.

Fixes: b049504e211e ("drm/amdkfd: Validate user queue svm memory residency")
Signed-off-by: Jesse Zhang <jesse.zh...@amd.com>
---
 drivers/gpu/drm/amd/amdkfd/kfd_queue.c | 23 +++++++++++++++++------
 1 file changed, 17 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_queue.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_queue.c
index ae3e7c54014a..6010a5d7a7eb 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_queue.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_queue.c
@@ -230,6 +230,15 @@ void kfd_queue_buffer_put(struct amdgpu_bo **bo)
        amdgpu_bo_unref(bo);
 }
 
+static void kfd_queue_put_bos(struct queue_properties *properties)
+{
+       kfd_queue_buffer_put(&properties->wptr_bo);
+       kfd_queue_buffer_put(&properties->rptr_bo);
+       kfd_queue_buffer_put(&properties->ring_bo);
+       kfd_queue_buffer_put(&properties->eop_buf_bo);
+       kfd_queue_buffer_put(&properties->cwsr_bo);
+}
+
 int kfd_queue_acquire_buffers(struct kfd_process_device *pdd, struct 
queue_properties *properties)
 {
        struct kfd_topology_device *topo_dev;
@@ -343,7 +352,13 @@ int kfd_queue_acquire_buffers(struct kfd_process_device 
*pdd, struct queue_prope
         * dropping the VM reservation.
         */
        kfd_queue_unref_bo_vas(pdd, properties);
-       kfd_queue_release_buffers(pdd, properties);
+       /*
+        * The SVM CWSR range is the last thing acquired and a failed
+        * kfd_queue_buffer_svm_get() takes nothing, so only the BOs are held
+        * here. Dropping the SVM reference would drop the one of another queue
+        * using the same CWSR address.
+        */
+       kfd_queue_put_bos(properties);
        return err;
 }
 
@@ -352,11 +367,7 @@ int kfd_queue_release_buffers(struct kfd_process_device 
*pdd, struct queue_prope
        struct kfd_topology_device *topo_dev;
        u64 total_cwsr_size;
 
-       kfd_queue_buffer_put(&properties->wptr_bo);
-       kfd_queue_buffer_put(&properties->rptr_bo);
-       kfd_queue_buffer_put(&properties->ring_bo);
-       kfd_queue_buffer_put(&properties->eop_buf_bo);
-       kfd_queue_buffer_put(&properties->cwsr_bo);
+       kfd_queue_put_bos(properties);
 
        topo_dev = kfd_topology_device_by_id(pdd->dev->id);
        if (!topo_dev)
-- 
2.50.1