radv/amdgpu: workaround a kernel bug when replacing sparse mappings

AMDGPU has a bug when clearing mappings for BOs that are always valid
in VM with OP_REPLACE.

See https://lists.freedesktop.org/archives/amd-gfx/2023-June/094648.html

The current workaround is to re-use OP_MAP/OP_UNMAP until all stable
kernels have the fix.

This partially reverts "radv/winsys: update sparse mappings with
OP_REPLACE instead of OP_MAP/OP_UNMAP".

Cc: mesa-stable
Signed-off-by: Samuel Pitoiset <samuel.pitoiset@gmail.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/23741>

diff --git a/src/amd/vulkan/winsys/amdgpu/radv_amdgpu_bo.c b/src/amd/vulkan/winsys/amdgpu/radv_amdgpu_bo.c
index 72771ab..9bca0d5 100644 (file)
--- a/src/amd/vulkan/winsys/amdgpu/radv_amdgpu_bo.c
+++ b/src/amd/vulkan/winsys/amdgpu/radv_amdgpu_bo.c
@@ -64,6 +64,42 @@ radv_amdgpu_bo_va_op(struct radv_amdgpu_winsys *ws, amdgpu_bo_handle bo, uint64_
    return amdgpu_bo_va_op_raw(ws->dev, bo, offset, size, addr, flags, ops);
 }
 
+static void
+radv_amdgpu_winsys_virtual_map(struct radv_amdgpu_winsys *ws, struct radv_amdgpu_winsys_bo *bo,
+                               const struct radv_amdgpu_map_range *range)
+{
+   uint64_t internal_flags = 0;
+   assert(range->size);
+
+   if (!range->bo) {
+      internal_flags |= AMDGPU_VM_PAGE_PRT;
+   }
+
+   int r = radv_amdgpu_bo_va_op(ws, range->bo ? range->bo->bo : NULL, range->bo_offset, range->size,
+                                range->offset + bo->base.va, 0, internal_flags, AMDGPU_VA_OP_MAP);
+   if (r)
+      abort();
+}
+
+static void
+radv_amdgpu_winsys_virtual_unmap(struct radv_amdgpu_winsys *ws, struct radv_amdgpu_winsys_bo *bo,
+                                 const struct radv_amdgpu_map_range *range)
+{
+   uint64_t internal_flags = 0;
+   assert(range->size);
+
+   if (!range->bo) {
+      /* Even though this is an unmap, if we don't set this flag,
+         AMDGPU is going to complain about the missing buffer. */
+      internal_flags |= AMDGPU_VM_PAGE_PRT;
+   }
+
+   int r = radv_amdgpu_bo_va_op(ws, range->bo ? range->bo->bo : NULL, range->bo_offset, range->size,
+                                range->offset + bo->base.va, 0, internal_flags, AMDGPU_VA_OP_UNMAP);
+   if (r)
+      abort();
+}
+
 static int
 bo_comparator(const void *ap, const void *bp)
 {
@@ -105,6 +141,11 @@ radv_amdgpu_winsys_rebuild_bo_list(struct radv_amdgpu_winsys_bo *bo)
    return VK_SUCCESS;
 }
 
+/**
+ * TODO: Use OP_REPLACE instead of OP_MAP/OP_UNMAP when
+ * https://lists.freedesktop.org/archives/amd-gfx/2023-June/094648.html is merged in all kernels
+ * we need to support.
+ */
 static VkResult
 radv_amdgpu_winsys_bo_virtual_bind(struct radeon_winsys *_ws, struct radeon_winsys_bo *_parent, uint64_t offset,
                                    uint64_t size, struct radeon_winsys_bo *_bo, uint64_t bo_offset)
@@ -116,26 +157,10 @@ radv_amdgpu_winsys_bo_virtual_bind(struct radeon_winsys *_ws, struct radeon_wins
    int first = 0, last;
    struct radv_amdgpu_map_range new_first, new_last;
    VkResult result;
-   int r;
 
    assert(parent->is_virtual);
    assert(!bo || !bo->is_virtual);
 
-   /* When the BO is NULL, AMDGPU will reset the PTE VA range to the initial state. Otherwise, it
-    * will first unmap all existing VA that overlap the requested range and then map.
-    */
-   if (bo) {
-      r = radv_amdgpu_bo_va_op(ws, bo->bo, bo_offset, size, parent->base.va + offset, 0, 0, AMDGPU_VA_OP_REPLACE);
-   } else {
-      r =
-         radv_amdgpu_bo_va_op(ws, NULL, 0, size, parent->base.va + offset, 0, AMDGPU_VM_PAGE_PRT, AMDGPU_VA_OP_REPLACE);
-   }
-
-   if (r) {
-      fprintf(stderr, "radv/amdgpu: Failed to replace a PRT VA region (%d).\n", r);
-      return VK_ERROR_OUT_OF_DEVICE_MEMORY;
-   }
-
    /* We have at most 2 new ranges (1 by the bind, and another one by splitting a range that
     * contains the newly bound range). */
    if (parent->range_capacity - parent->range_count < 2) {
@@ -166,6 +191,7 @@ radv_amdgpu_winsys_bo_virtual_bind(struct radeon_winsys *_ws, struct radeon_wins
     * whether to not create the corresponding split part. */
    bool remove_first = parent->ranges[first].offset == offset;
    bool remove_last = parent->ranges[last].offset + parent->ranges[last].size == offset + size;
+   bool unmapped_first = false;
 
    assert(parent->ranges[first].offset <= offset);
    assert(parent->ranges[last].offset + parent->ranges[last].size >= offset + size);
@@ -189,6 +215,11 @@ radv_amdgpu_winsys_bo_virtual_bind(struct radeon_winsys *_ws, struct radeon_wins
    range_count_delta = 1 - (last - first + 1) + !remove_first + !remove_last;
    new_idx = first + !remove_first;
 
+   /* Any range between first and last is going to be entirely covered by the new range so just
+    * unmap them. */
+   for (int i = first + 1; i < last; ++i)
+      radv_amdgpu_winsys_virtual_unmap(ws, parent, parent->ranges + i);
+
    /* If the first/last range are not left alone we unmap then and optionally map
     * them again after modifications. Not that this implicitly can do the splitting
     * if first == last. */
@@ -196,16 +227,24 @@ radv_amdgpu_winsys_bo_virtual_bind(struct radeon_winsys *_ws, struct radeon_wins
    new_last = parent->ranges[last];
 
    if (parent->ranges[first].offset + parent->ranges[first].size > offset || remove_first) {
+      radv_amdgpu_winsys_virtual_unmap(ws, parent, parent->ranges + first);
+      unmapped_first = true;
+
       if (!remove_first) {
          new_first.size = offset - new_first.offset;
+         radv_amdgpu_winsys_virtual_map(ws, parent, &new_first);
       }
    }
 
    if (parent->ranges[last].offset < offset + size || remove_last) {
+      if (first != last || !unmapped_first)
+         radv_amdgpu_winsys_virtual_unmap(ws, parent, parent->ranges + last);
+
       if (!remove_last) {
          new_last.size -= offset + size - new_last.offset;
          new_last.bo_offset += (offset + size - new_last.offset);
          new_last.offset = offset + size;
+         radv_amdgpu_winsys_virtual_map(ws, parent, &new_last);
       }
    }
 
@@ -225,6 +264,8 @@ radv_amdgpu_winsys_bo_virtual_bind(struct radeon_winsys *_ws, struct radeon_wins
    parent->ranges[new_idx].bo = bo;
    parent->ranges[new_idx].bo_offset = bo_offset;
 
+   radv_amdgpu_winsys_virtual_map(ws, parent, parent->ranges + new_idx);
+
    parent->range_count += range_count_delta;
 
    result = radv_amdgpu_winsys_rebuild_bo_list(parent);
@@ -404,14 +445,7 @@ radv_amdgpu_winsys_bo_create(struct radeon_winsys *_ws, uint64_t size, unsigned
       bo->ranges[0].bo = NULL;
       bo->ranges[0].bo_offset = 0;
 
-      /* Reserve a PRT VA region. */
-      r = radv_amdgpu_bo_va_op(ws, NULL, 0, size, bo->base.va, 0, AMDGPU_VM_PAGE_PRT, AMDGPU_VA_OP_MAP);
-      if (r) {
-         fprintf(stderr, "radv/amdgpu: Failed to reserve a PRT VA region (%d).\n", r);
-         result = VK_ERROR_OUT_OF_DEVICE_MEMORY;
-         goto error_ranges_alloc;
-      }
-
+      radv_amdgpu_winsys_virtual_map(ws, bo, bo->ranges);
       radv_amdgpu_log_bo(ws, bo, false);
 
       *out_bo = (struct radeon_winsys_bo *)bo;