drm/xe: Use NULL PTEs as scratch PTEs
authorThomas Hellström <thomas.hellstrom@linux.intel.com>
Sat, 9 Dec 2023 15:18:42 +0000 (16:18 +0100)
committerRodrigo Vivi <rodrigo.vivi@intel.com>
Thu, 21 Dec 2023 16:45:28 +0000 (11:45 -0500)
Currently scratch PTEs are write-enabled and points to a single scratch
page. This has the side effect that buggy applications with out-of-bounds
memory accesses may not notice the bad access since what's written may
be read back.

Instead use NULL PTEs as scratch PTEs. These always return 0 when reading,
and writing has no effect. As a slight benefit, we can also use huge NULL
PTEs.

One drawback pointed out is that debugging may be hampered since previously
when inspecting the content of the scratch page, it might be possible to
detect writes to out-of-bound addresses and possibly also
from where the out-of-bounds address originated. However since the scratch
page-table structure is kept, it will be easy to add back the single
RW-enabled scratch page under a debug define if needed.

Also update the kerneldoc accordingly and move the function to create the
scratch page-tables from xe_pt.c to xe_pt.h since it is accessing
vm structure internals and this also makes it possible to make it static.

v2:
- Don't try to encode scratch PTEs larger than 1GiB.
- Move xe_pt_create_scratch(), Update kerneldoc.
v3:
- Rebase.

Cc: Brian Welty <brian.welty@intel.com>
Cc: Matt Roper <matthew.d.roper@intel.com>
Signed-off-by: Thomas Hellström <thomas.hellstrom@linux.intel.com>
Acked-by: Lucas De Marchi <lucas.demarchi@intel.com> #for general direction.
Reviewed-by: Brian Welty <brian.welty@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20231209151843.7903-3-thomas.hellstrom@linux.intel.com
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
drivers/gpu/drm/xe/xe_pt.c
drivers/gpu/drm/xe/xe_pt.h
drivers/gpu/drm/xe/xe_vm.c
drivers/gpu/drm/xe/xe_vm.h
drivers/gpu/drm/xe/xe_vm_types.h

index 46ef9df34a2e3ed64caea9c577c0c85ba3ee8e82..de1030a47588371b0cc71f5b69bee8f0257e2625 100644 (file)
@@ -50,17 +50,19 @@ static struct xe_pt *xe_pt_entry(struct xe_pt_dir *pt_dir, unsigned int index)
 static u64 __xe_pt_empty_pte(struct xe_tile *tile, struct xe_vm *vm,
                             unsigned int level)
 {
-       u16 pat_index = tile_to_xe(tile)->pat.idx[XE_CACHE_WB];
+       struct xe_device *xe = tile_to_xe(tile);
+       u16 pat_index = xe->pat.idx[XE_CACHE_WB];
        u8 id = tile->id;
 
-       if (!vm->scratch_bo[id])
+       if (!xe_vm_has_scratch(vm))
                return 0;
 
-       if (level > 0)
+       if (level > MAX_HUGEPTE_LEVEL)
                return vm->pt_ops->pde_encode_bo(vm->scratch_pt[id][level - 1]->bo,
                                                 0, pat_index);
 
-       return vm->pt_ops->pte_encode_bo(vm->scratch_bo[id], 0, pat_index, 0);
+       return vm->pt_ops->pte_encode_addr(xe, 0, pat_index, level, IS_DGFX(xe), 0) |
+               XE_PTE_NULL;
 }
 
 /**
@@ -135,7 +137,7 @@ void xe_pt_populate_empty(struct xe_tile *tile, struct xe_vm *vm,
        u64 empty;
        int i;
 
-       if (!vm->scratch_bo[tile->id]) {
+       if (!xe_vm_has_scratch(vm)) {
                /*
                 * FIXME: Some memory is allocated already allocated to zero?
                 * Find out which memory that is and avoid this memset...
@@ -194,57 +196,6 @@ void xe_pt_destroy(struct xe_pt *pt, u32 flags, struct llist_head *deferred)
        kfree(pt);
 }
 
-/**
- * xe_pt_create_scratch() - Setup a scratch memory pagetable tree for the
- * given tile and vm.
- * @xe: xe device.
- * @tile: tile to set up for.
- * @vm: vm to set up for.
- *
- * Sets up a pagetable tree with one page-table per level and a single
- * leaf bo. All pagetable entries point to the single page-table or,
- * for L0, the single bo one level below.
- *
- * Return: 0 on success, negative error code on error.
- */
-int xe_pt_create_scratch(struct xe_device *xe, struct xe_tile *tile,
-                        struct xe_vm *vm)
-{
-       u8 id = tile->id;
-       unsigned int flags;
-       int i;
-
-       /*
-        * So we don't need to worry about 64K TLB hints when dealing with
-        * scratch entires, rather keep the scratch page in system memory on
-        * platforms where 64K pages are needed for VRAM.
-        */
-       flags = XE_BO_CREATE_PINNED_BIT;
-       if (vm->flags & XE_VM_FLAG_64K)
-               flags |= XE_BO_CREATE_SYSTEM_BIT;
-       else
-               flags |= XE_BO_CREATE_VRAM_IF_DGFX(tile);
-
-       vm->scratch_bo[id] = xe_bo_create_pin_map(xe, tile, vm, SZ_4K,
-                                                 ttm_bo_type_kernel,
-                                                 flags);
-       if (IS_ERR(vm->scratch_bo[id]))
-               return PTR_ERR(vm->scratch_bo[id]);
-
-       xe_map_memset(vm->xe, &vm->scratch_bo[id]->vmap, 0, 0,
-                     vm->scratch_bo[id]->size);
-
-       for (i = 0; i < vm->pt_root[id]->level; i++) {
-               vm->scratch_pt[id][i] = xe_pt_create(vm, tile, i);
-               if (IS_ERR(vm->scratch_pt[id][i]))
-                       return PTR_ERR(vm->scratch_pt[id][i]);
-
-               xe_pt_populate_empty(tile, vm, vm->scratch_pt[id][i]);
-       }
-
-       return 0;
-}
-
 /**
  * DOC: Pagetable building
  *
@@ -1289,7 +1240,7 @@ __xe_pt_bind_vma(struct xe_tile *tile, struct xe_vma *vma, struct xe_exec_queue
         * it needs to be done here.
         */
        if ((rebind && !xe_vm_in_lr_mode(vm) && !vm->batch_invalidate_tlb) ||
-           (!rebind && vm->scratch_bo[tile->id] && xe_vm_in_preempt_fence_mode(vm))) {
+           (!rebind && xe_vm_has_scratch(vm) && xe_vm_in_preempt_fence_mode(vm))) {
                ifence = kzalloc(sizeof(*ifence), GFP_KERNEL);
                if (!ifence)
                        return ERR_PTR(-ENOMEM);
index ba2f3325c84de7570592538a265d1159d32bdcf7..71a4fbfcff43a4db53d0ff138cdfffd194cffdbe 100644 (file)
@@ -29,9 +29,6 @@ unsigned int xe_pt_shift(unsigned int level);
 struct xe_pt *xe_pt_create(struct xe_vm *vm, struct xe_tile *tile,
                           unsigned int level);
 
-int xe_pt_create_scratch(struct xe_device *xe, struct xe_tile *tile,
-                        struct xe_vm *vm);
-
 void xe_pt_populate_empty(struct xe_tile *tile, struct xe_vm *vm,
                          struct xe_pt *pt);
 
index d589beb99fe69cbb43e01b1aa4220b5d876e3a05..e190469ec03aadd736654f078b8b90a380f34bda 100644 (file)
@@ -1348,6 +1348,57 @@ static const struct xe_pt_ops xelp_pt_ops = {
 
 static void vm_destroy_work_func(struct work_struct *w);
 
+/**
+ * xe_vm_create_scratch() - Setup a scratch memory pagetable tree for the
+ * given tile and vm.
+ * @xe: xe device.
+ * @tile: tile to set up for.
+ * @vm: vm to set up for.
+ *
+ * Sets up a pagetable tree with one page-table per level and a single
+ * leaf PTE. All pagetable entries point to the single page-table or,
+ * for MAX_HUGEPTE_LEVEL, a NULL huge PTE returning 0 on read and
+ * writes become NOPs.
+ *
+ * Return: 0 on success, negative error code on error.
+ */
+static int xe_vm_create_scratch(struct xe_device *xe, struct xe_tile *tile,
+                               struct xe_vm *vm)
+{
+       u8 id = tile->id;
+       int i;
+
+       for (i = MAX_HUGEPTE_LEVEL; i < vm->pt_root[id]->level; i++) {
+               vm->scratch_pt[id][i] = xe_pt_create(vm, tile, i);
+               if (IS_ERR(vm->scratch_pt[id][i]))
+                       return PTR_ERR(vm->scratch_pt[id][i]);
+
+               xe_pt_populate_empty(tile, vm, vm->scratch_pt[id][i]);
+       }
+
+       return 0;
+}
+
+static void xe_vm_free_scratch(struct xe_vm *vm)
+{
+       struct xe_tile *tile;
+       u8 id;
+
+       if (!xe_vm_has_scratch(vm))
+               return;
+
+       for_each_tile(tile, vm->xe, id) {
+               u32 i;
+
+               if (!vm->pt_root[id])
+                       continue;
+
+               for (i = MAX_HUGEPTE_LEVEL; i < vm->pt_root[id]->level; ++i)
+                       if (vm->scratch_pt[id][i])
+                               xe_pt_destroy(vm->scratch_pt[id][i], vm->flags, NULL);
+       }
+}
+
 struct xe_vm *xe_vm_create(struct xe_device *xe, u32 flags)
 {
        struct drm_gem_object *vm_resv_obj;
@@ -1424,12 +1475,12 @@ struct xe_vm *xe_vm_create(struct xe_device *xe, u32 flags)
                }
        }
 
-       if (flags & XE_VM_FLAG_SCRATCH_PAGE) {
+       if (xe_vm_has_scratch(vm)) {
                for_each_tile(tile, xe, id) {
                        if (!vm->pt_root[id])
                                continue;
 
-                       err = xe_pt_create_scratch(xe, tile, vm);
+                       err = xe_vm_create_scratch(xe, tile, vm);
                        if (err)
                                goto err_unlock_close;
                }
@@ -1575,16 +1626,9 @@ void xe_vm_close_and_put(struct xe_vm *vm)
         * install a fence to resv. Hence it's safe to
         * destroy the pagetables immediately.
         */
-       for_each_tile(tile, xe, id) {
-               if (vm->scratch_bo[id]) {
-                       u32 i;
+       xe_vm_free_scratch(vm);
 
-                       xe_bo_unpin(vm->scratch_bo[id]);
-                       xe_bo_put(vm->scratch_bo[id]);
-                       for (i = 0; i < vm->pt_root[id]->level; i++)
-                               xe_pt_destroy(vm->scratch_pt[id][i], vm->flags,
-                                             NULL);
-               }
+       for_each_tile(tile, xe, id) {
                if (vm->pt_root[id]) {
                        xe_pt_destroy(vm->pt_root[id], vm->flags, NULL);
                        vm->pt_root[id] = NULL;
index 12bb5d79487f234394e8ab565a315100c47273ca..a1907544cc4f4de69b0cb06dda72858e0d80a2e4 100644 (file)
@@ -63,6 +63,17 @@ static inline bool xe_vm_is_closed_or_banned(struct xe_vm *vm)
 struct xe_vma *
 xe_vm_find_overlapping_vma(struct xe_vm *vm, u64 start, u64 range);
 
+/**
+ * xe_vm_has_scratch() - Whether the vm is configured for scratch PTEs
+ * @vm: The vm
+ *
+ * Return: whether the vm populates unmapped areas with scratch PTEs
+ */
+static inline bool xe_vm_has_scratch(const struct xe_vm *vm)
+{
+       return vm->flags & XE_VM_FLAG_SCRATCH_PAGE;
+}
+
 static inline struct xe_vm *gpuva_to_vm(struct drm_gpuva *gpuva)
 {
        return container_of(gpuva->vm, struct xe_vm, gpuvm);
index e70ec6b2fabed472e98204f38723807f104b38eb..15471025a44f6deeb5d4700791a58cc73015f01a 100644 (file)
@@ -151,7 +151,6 @@ struct xe_vm {
        u64 size;
 
        struct xe_pt *pt_root[XE_MAX_TILES_PER_DEVICE];
-       struct xe_bo *scratch_bo[XE_MAX_TILES_PER_DEVICE];
        struct xe_pt *scratch_pt[XE_MAX_TILES_PER_DEVICE][XE_VM_MAX_LEVEL];
 
        /**