* [PATCH 1/4] drm/i915: Allow i915 to manage the vma offset nodes instead of drm core
@ 2019-11-15 11:45 ` Abdiel Janulgue
0 siblings, 0 replies; 28+ messages in thread
From: Abdiel Janulgue @ 2019-11-15 11:45 UTC (permalink / raw)
To: intel-gfx; +Cc: Matthew Auld
Have i915 replace the core drm_gem_mmap implementation to overcome its
limitation in having only a single mmap offset node per gem object.
This change allows us to have multiple mmap offsets per object and
enables a mmapping instance to use unique fault-handlers per user vma.
This allows i915 to store extra data within vma->vm_private_data and
assign the pagefault ops for each mmap instance allowing objects to use
multiple fault handlers depending on its backing storage.
v2:
- Fix race condition exposed by gem_mmap_gtt@close-race. Simplify
lifetime management of the mmap offset objects be ensuring it is
owned by the parent gem object instead of refcounting.
- Track mmo used by fencing to Avoid locking when revoking mmaps
during GPU reset.
- Rebase.
v3:
- Simplify mmo tracking
v4:
- use vma->mmo in __i915_gem_object_release_mmap_gtt
v5:
- Revoke CPU mmaps on i915_gem_object_unbind() since unlike GTT
mmaps, they don't have bound i915_vmas objects. Rebase.
v6: Minor tweaks, header re-org (Chris)
Signed-off-by: Abdiel Janulgue <abdiel.janulgue@linux.intel.com>
Cc: Matthew Auld <matthew.auld@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
---
drivers/gpu/drm/i915/gem/i915_gem_domain.c | 3 +-
drivers/gpu/drm/i915/gem/i915_gem_mman.c | 243 +++++++++++++++---
drivers/gpu/drm/i915/gem/i915_gem_mman.h | 28 ++
drivers/gpu/drm/i915/gem/i915_gem_object.c | 14 +
drivers/gpu/drm/i915/gem/i915_gem_object.h | 7 +-
.../gpu/drm/i915/gem/i915_gem_object_types.h | 19 ++
drivers/gpu/drm/i915/gem/i915_gem_pages.c | 3 +
drivers/gpu/drm/i915/gem/i915_gem_tiling.c | 1 +
.../drm/i915/gem/selftests/i915_gem_mman.c | 21 +-
drivers/gpu/drm/i915/gt/intel_reset.c | 7 +-
drivers/gpu/drm/i915/i915_drv.c | 10 +-
drivers/gpu/drm/i915/i915_drv.h | 3 -
drivers/gpu/drm/i915/i915_gem.c | 3 +-
drivers/gpu/drm/i915/i915_getparam.c | 1 +
drivers/gpu/drm/i915/i915_vma.c | 3 +-
drivers/gpu/drm/i915/i915_vma.h | 3 +
16 files changed, 313 insertions(+), 56 deletions(-)
create mode 100644 drivers/gpu/drm/i915/gem/i915_gem_mman.h
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_domain.c b/drivers/gpu/drm/i915/gem/i915_gem_domain.c
index e2af63af67ad..fd1c6f12b77a 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_domain.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_domain.c
@@ -13,6 +13,7 @@
#include "i915_gem_object.h"
#include "i915_vma.h"
#include "i915_gem_lmem.h"
+#include "i915_gem_mman.h"
static void __i915_gem_object_flush_for_display(struct drm_i915_gem_object *obj)
{
@@ -255,7 +256,7 @@ int i915_gem_object_set_cache_level(struct drm_i915_gem_object *obj,
}
if (obj->userfault_count)
- __i915_gem_object_release_mmap(obj);
+ __i915_gem_object_release_mmap_gtt(obj);
/*
* As we no longer need a fence for GTT access,
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_mman.c b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
index d60973603cc1..e602dfb44532 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_mman.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
@@ -14,6 +14,7 @@
#include "i915_gem_gtt.h"
#include "i915_gem_ioctls.h"
#include "i915_gem_object.h"
+#include "i915_gem_mman.h"
#include "i915_trace.h"
#include "i915_vma.h"
@@ -219,7 +220,8 @@ vm_fault_t i915_gem_fault(struct vm_fault *vmf)
{
#define MIN_CHUNK_PAGES (SZ_1M >> PAGE_SHIFT)
struct vm_area_struct *area = vmf->vma;
- struct drm_i915_gem_object *obj = to_intel_bo(area->vm_private_data);
+ struct i915_mmap_offset *priv = area->vm_private_data;
+ struct drm_i915_gem_object *obj = priv->obj;
struct drm_device *dev = obj->base.dev;
struct drm_i915_private *i915 = to_i915(dev);
struct intel_runtime_pm *rpm = &i915->runtime_pm;
@@ -312,6 +314,9 @@ vm_fault_t i915_gem_fault(struct vm_fault *vmf)
list_add(&obj->userfault_link, &i915->ggtt.userfault_list);
mutex_unlock(&i915->ggtt.vm.mutex);
+ /* Track the mmo associated with the fenced vma */
+ vma->mmo = priv;
+
if (IS_ACTIVE(CONFIG_DRM_I915_USERFAULT_AUTOSUSPEND))
intel_wakeref_auto(&i915->ggtt.userfault_wakeref,
msecs_to_jiffies_timeout(CONFIG_DRM_I915_USERFAULT_AUTOSUSPEND));
@@ -358,28 +363,19 @@ vm_fault_t i915_gem_fault(struct vm_fault *vmf)
}
}
-void __i915_gem_object_release_mmap(struct drm_i915_gem_object *obj)
+void __i915_gem_object_release_mmap_gtt(struct drm_i915_gem_object *obj)
{
struct i915_vma *vma;
GEM_BUG_ON(!obj->userfault_count);
- obj->userfault_count = 0;
- list_del(&obj->userfault_link);
- drm_vma_node_unmap(&obj->base.vma_node,
- obj->base.dev->anon_inode->i_mapping);
-
for_each_ggtt_vma(vma, obj)
- i915_vma_unset_userfault(vma);
+ i915_vma_revoke_mmap(vma);
+
+ GEM_BUG_ON(obj->userfault_count);
}
/**
- * i915_gem_object_release_mmap - remove physical page mappings
- * @obj: obj in question
- *
- * Preserve the reservation of the mmapping with the DRM core code, but
- * relinquish ownership of the pages back to the system.
- *
* It is vital that we remove the page mapping if we have mapped a tiled
* object through the GTT and then lose the fence register due to
* resource pressure. Similarly if the object has been moved out of the
@@ -387,7 +383,7 @@ void __i915_gem_object_release_mmap(struct drm_i915_gem_object *obj)
* mapping will then trigger a page fault on the next user access, allowing
* fixup by i915_gem_fault().
*/
-void i915_gem_object_release_mmap(struct drm_i915_gem_object *obj)
+static void i915_gem_object_release_mmap_gtt(struct drm_i915_gem_object *obj)
{
struct drm_i915_private *i915 = to_i915(obj->base.dev);
intel_wakeref_t wakeref;
@@ -406,7 +402,7 @@ void i915_gem_object_release_mmap(struct drm_i915_gem_object *obj)
if (!obj->userfault_count)
goto out;
- __i915_gem_object_release_mmap(obj);
+ __i915_gem_object_release_mmap_gtt(obj);
/* Ensure that the CPU's PTE are revoked and there are not outstanding
* memory transactions from userspace before we return. The TLB
@@ -422,15 +418,63 @@ void i915_gem_object_release_mmap(struct drm_i915_gem_object *obj)
intel_runtime_pm_put(&i915->runtime_pm, wakeref);
}
-static int create_mmap_offset(struct drm_i915_gem_object *obj)
+void i915_gem_object_release_mmap_offset(struct drm_i915_gem_object *obj)
+{
+ struct i915_mmap_offset *mmo;
+
+ mutex_lock(&obj->mmo_lock);
+ list_for_each_entry(mmo, &obj->mmap_offsets, offset) {
+ /* vma_node_unmap for GTT mmaps handled already in
+ * __i915_gem_object_release_mmap_gtt
+ */
+ if (mmo->mmap_type != I915_MMAP_TYPE_GTT)
+ drm_vma_node_unmap(&mmo->vma_node,
+ obj->base.dev->anon_inode->i_mapping);
+ }
+ mutex_unlock(&obj->mmo_lock);
+}
+
+/**
+ * i915_gem_object_release_mmap - remove physical page mappings
+ * @obj: obj in question
+ *
+ * Preserve the reservation of the mmapping with the DRM core code, but
+ * relinquish ownership of the pages back to the system.
+ */
+void i915_gem_object_release_mmap(struct drm_i915_gem_object *obj)
+{
+ i915_gem_object_release_mmap_gtt(obj);
+ i915_gem_object_release_mmap_offset(obj);
+}
+
+static void init_mmap_offset(struct drm_i915_gem_object *obj,
+ struct i915_mmap_offset *mmo)
+{
+ mutex_lock(&obj->mmo_lock);
+ list_add(&mmo->offset, &obj->mmap_offsets);
+ mutex_unlock(&obj->mmo_lock);
+
+ mmo->obj = obj;
+ mmo->dev = obj->base.dev;
+}
+
+static int create_mmap_offset(struct drm_i915_gem_object *obj,
+ struct i915_mmap_offset *mmo)
{
struct drm_i915_private *i915 = to_i915(obj->base.dev);
struct intel_gt *gt = &i915->gt;
+ struct drm_device *dev = obj->base.dev;
int err;
- err = drm_gem_create_mmap_offset(&obj->base);
- if (likely(!err))
+ drm_vma_node_reset(&mmo->vma_node);
+ if (mmo->file)
+ drm_vma_node_allow(&mmo->vma_node, mmo->file);
+ err = drm_vma_offset_add(dev->vma_offset_manager, &mmo->vma_node,
+ obj->base.size / PAGE_SIZE);
+ if (likely(!err)) {
+ init_mmap_offset(obj, mmo);
return 0;
+ }
/* Attempt to reap some mmap space from dead objects */
err = intel_gt_retire_requests_timeout(gt, MAX_SCHEDULE_TIMEOUT);
@@ -438,34 +482,56 @@ static int create_mmap_offset(struct drm_i915_gem_object *obj)
return err;
i915_gem_drain_freed_objects(i915);
- return drm_gem_create_mmap_offset(&obj->base);
+ err = drm_vma_offset_add(dev->vma_offset_manager, &mmo->vma_node,
+ obj->base.size / PAGE_SIZE);
+ if (err)
+ return err;
+
+ init_mmap_offset(obj, mmo);
+ return 0;
}
-int
-i915_gem_mmap_gtt(struct drm_file *file,
- struct drm_device *dev,
- u32 handle,
- u64 *offset)
+static int
+__assign_gem_object_mmap_data(struct drm_file *file,
+ u32 handle,
+ enum i915_mmap_type mmap_type,
+ u64 *offset)
{
struct drm_i915_gem_object *obj;
+ struct i915_mmap_offset *mmo;
int ret;
- if (!i915_ggtt_has_aperture(&to_i915(dev)->ggtt))
- return -ENODEV;
-
obj = i915_gem_object_lookup(file, handle);
if (!obj)
return -ENOENT;
- if (i915_gem_object_never_bind_ggtt(obj)) {
+ if (mmap_type == I915_MMAP_TYPE_GTT &&
+ i915_gem_object_never_bind_ggtt(obj)) {
ret = -ENODEV;
goto out;
}
- ret = create_mmap_offset(obj);
- if (ret == 0)
- *offset = drm_vma_node_offset_addr(&obj->base.vma_node);
+ if (mmap_type != I915_MMAP_TYPE_GTT &&
+ !i915_gem_object_has_struct_page(obj)) {
+ ret = -ENODEV;
+ goto out;
+ }
+
+ mmo = kzalloc(sizeof(*mmo), GFP_KERNEL);
+ if (!mmo) {
+ ret = -ENOMEM;
+ goto out;
+ }
+
+ mmo->file = file;
+ ret = create_mmap_offset(obj, mmo);
+ if (ret) {
+ kfree(mmo);
+ goto out;
+ }
+ mmo->mmap_type = mmap_type;
+ *offset = drm_vma_node_offset_addr(&mmo->vma_node);
out:
i915_gem_object_put(obj);
return ret;
@@ -492,7 +558,118 @@ i915_gem_mmap_gtt_ioctl(struct drm_device *dev, void *data,
{
struct drm_i915_gem_mmap_gtt *args = data;
- return i915_gem_mmap_gtt(file, dev, args->handle, &args->offset);
+ return __assign_gem_object_mmap_data(file, args->handle,
+ I915_MMAP_TYPE_GTT,
+ &args->offset);
+}
+
+void i915_mmap_offset_destroy(struct i915_mmap_offset *mmo, struct mutex *mutex)
+{
+ if (mmo->file)
+ drm_vma_node_revoke(&mmo->vma_node, mmo->file);
+ drm_vma_offset_remove(mmo->dev->vma_offset_manager, &mmo->vma_node);
+
+ mutex_lock(mutex);
+ list_del(&mmo->offset);
+ mutex_unlock(mutex);
+
+ kfree(mmo);
+}
+
+static void i915_gem_vm_open(struct vm_area_struct *vma)
+{
+ struct i915_mmap_offset *mmo = vma->vm_private_data;
+ struct drm_i915_gem_object *obj = mmo->obj;
+
+ GEM_BUG_ON(!obj);
+ i915_gem_object_get(obj);
+}
+
+static void i915_gem_vm_close(struct vm_area_struct *vma)
+{
+ struct i915_mmap_offset *mmo = vma->vm_private_data;
+ struct drm_i915_gem_object *obj = mmo->obj;
+
+ GEM_BUG_ON(!obj);
+ i915_gem_object_put(obj);
+}
+
+static const struct vm_operations_struct i915_gem_gtt_vm_ops = {
+ .fault = i915_gem_fault,
+ .open = i915_gem_vm_open,
+ .close = i915_gem_vm_close,
+};
+
+/* This overcomes the limitation in drm_gem_mmap's assignment of a
+ * drm_gem_object as the vma->vm_private_data. Since we need to
+ * be able to resolve multiple mmap offsets which could be tied
+ * to a single gem object.
+ */
+int i915_gem_mmap(struct file *filp, struct vm_area_struct *vma)
+{
+ struct drm_vma_offset_node *node;
+ struct drm_file *priv = filp->private_data;
+ struct drm_device *dev = priv->minor->dev;
+ struct i915_mmap_offset *mmo = NULL;
+ struct drm_gem_object *obj = NULL;
+
+ if (drm_dev_is_unplugged(dev))
+ return -ENODEV;
+
+ drm_vma_offset_lock_lookup(dev->vma_offset_manager);
+ node = drm_vma_offset_exact_lookup_locked(dev->vma_offset_manager,
+ vma->vm_pgoff,
+ vma_pages(vma));
+ if (likely(node)) {
+ mmo = container_of(node, struct i915_mmap_offset,
+ vma_node);
+ /*
+ * In our dependency chain, the drm_vma_offset_node
+ * depends on the validity of the mmo, which depends on
+ * the gem object. However the only reference we have
+ * at this point is the mmo (as the parent of the node).
+ * Try to check if the gem object was at least cleared.
+ */
+ if (!mmo || !mmo->obj) {
+ drm_vma_offset_unlock_lookup(dev->vma_offset_manager);
+ return -EINVAL;
+ }
+ /*
+ * Skip 0-refcnted objects as it is in the process of being
+ * destroyed and will be invalid when the vma manager lock
+ * is released.
+ */
+ obj = &mmo->obj->base;
+ if (!kref_get_unless_zero(&obj->refcount))
+ obj = NULL;
+
+ }
+ drm_vma_offset_unlock_lookup(dev->vma_offset_manager);
+
+ if (!obj)
+ return -EINVAL;
+
+ if (!drm_vma_node_is_allowed(node, priv)) {
+ drm_gem_object_put_unlocked(obj);
+ return -EACCES;
+ }
+
+ if (to_intel_bo(obj)->readonly) {
+ if (vma->vm_flags & VM_WRITE) {
+ drm_gem_object_put_unlocked(obj);
+ return -EINVAL;
+ }
+ vma->vm_flags &= ~VM_MAYWRITE;
+ }
+
+ vma->vm_flags |= VM_PFNMAP | VM_DONTEXPAND | VM_DONTDUMP;
+ vma->vm_page_prot = pgprot_writecombine(vm_get_page_prot(vma->vm_flags));
+ vma->vm_page_prot = pgprot_decrypted(vma->vm_page_prot);
+ vma->vm_private_data = mmo;
+
+ vma->vm_ops = &i915_gem_gtt_vm_ops;
+
+ return 0;
}
#if IS_ENABLED(CONFIG_DRM_I915_SELFTEST)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_mman.h b/drivers/gpu/drm/i915/gem/i915_gem_mman.h
new file mode 100644
index 000000000000..25a3c4d6cd65
--- /dev/null
+++ b/drivers/gpu/drm/i915/gem/i915_gem_mman.h
@@ -0,0 +1,28 @@
+/*
+ * SPDX-License-Identifier: MIT
+ *
+ * Copyright © 2019 Intel Corporation
+ */
+
+#ifndef __I915_GEM_MMAN_H__
+#define __I915_GEM_MMAN_H__
+
+#include <linux/mm_types.h>
+#include <linux/types.h>
+
+struct drm_device;
+struct drm_file;
+struct drm_i915_gem_object;
+struct file;
+struct i915_mmap_offset;
+struct mutex;
+
+int i915_gem_mmap(struct file *filp, struct vm_area_struct *vma);
+vm_fault_t i915_gem_fault(struct vm_fault *vmf);
+void i915_mmap_offset_destroy(struct i915_mmap_offset *mmo, struct mutex *mutex);
+
+void __i915_gem_object_release_mmap_gtt(struct drm_i915_gem_object *obj);
+void i915_gem_object_release_mmap(struct drm_i915_gem_object *obj);
+void i915_gem_object_release_mmap_offset(struct drm_i915_gem_object *obj);
+
+#endif
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.c b/drivers/gpu/drm/i915/gem/i915_gem_object.c
index db103d3c8760..c4f4f10c852d 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_object.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_object.c
@@ -29,6 +29,7 @@
#include "i915_drv.h"
#include "i915_gem_clflush.h"
#include "i915_gem_context.h"
+#include "i915_gem_mman.h"
#include "i915_gem_object.h"
#include "i915_globals.h"
#include "i915_trace.h"
@@ -61,6 +62,9 @@ void i915_gem_object_init(struct drm_i915_gem_object *obj,
INIT_LIST_HEAD(&obj->lut_list);
+ mutex_init(&obj->mmo_lock);
+ INIT_LIST_HEAD(&obj->mmap_offsets);
+
init_rcu_head(&obj->rcu);
obj->ops = ops;
@@ -158,6 +162,8 @@ static void __i915_gem_free_objects(struct drm_i915_private *i915,
wakeref = intel_runtime_pm_get(&i915->runtime_pm);
llist_for_each_entry_safe(obj, on, freed, freed) {
+ struct i915_mmap_offset *mmo, *mn;
+
trace_i915_gem_object_destroy(obj);
if (!list_empty(&obj->vma.list)) {
@@ -183,6 +189,14 @@ static void __i915_gem_free_objects(struct drm_i915_private *i915,
spin_unlock(&obj->vma.lock);
}
+ i915_gem_object_release_mmap(obj);
+
+ list_for_each_entry_safe(mmo, mn, &obj->mmap_offsets, offset) {
+ mmo->obj = NULL;
+ i915_mmap_offset_destroy(mmo, &obj->mmo_lock);
+ }
+
+ GEM_BUG_ON(!list_empty(&obj->mmap_offsets));
GEM_BUG_ON(atomic_read(&obj->bind_count));
GEM_BUG_ON(obj->userfault_count);
GEM_BUG_ON(!list_empty(&obj->lut_list));
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.h b/drivers/gpu/drm/i915/gem/i915_gem_object.h
index e5750d506cc9..9b61f18606e0 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_object.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_object.h
@@ -132,13 +132,13 @@ void i915_gem_object_unlock_fence(struct drm_i915_gem_object *obj,
static inline void
i915_gem_object_set_readonly(struct drm_i915_gem_object *obj)
{
- obj->base.vma_node.readonly = true;
+ obj->readonly = true;
}
static inline bool
i915_gem_object_is_readonly(const struct drm_i915_gem_object *obj)
{
- return obj->base.vma_node.readonly;
+ return obj->readonly;
}
static inline bool
@@ -387,9 +387,6 @@ static inline void i915_gem_object_unpin_map(struct drm_i915_gem_object *obj)
i915_gem_object_unpin_pages(obj);
}
-void __i915_gem_object_release_mmap(struct drm_i915_gem_object *obj);
-void i915_gem_object_release_mmap(struct drm_i915_gem_object *obj);
-
void
i915_gem_object_flush_write_domain(struct drm_i915_gem_object *obj,
unsigned int flush_domains);
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object_types.h b/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
index 15f8297dc34e..632d6b804cfb 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
@@ -63,6 +63,20 @@ struct drm_i915_gem_object_ops {
void (*release)(struct drm_i915_gem_object *obj);
};
+enum i915_mmap_type {
+ I915_MMAP_TYPE_GTT = 0,
+};
+
+struct i915_mmap_offset {
+ struct drm_device *dev;
+ struct drm_vma_offset_node vma_node;
+ struct drm_i915_gem_object *obj;
+ struct drm_file *file;
+ enum i915_mmap_type mmap_type;
+
+ struct list_head offset;
+};
+
struct drm_i915_gem_object {
struct drm_gem_object base;
@@ -118,6 +132,11 @@ struct drm_i915_gem_object {
unsigned int userfault_count;
struct list_head userfault_link;
+ /* Protects access to mmap offsets */
+ struct mutex mmo_lock;
+ struct list_head mmap_offsets;
+ bool readonly:1;
+
I915_SELFTEST_DECLARE(struct list_head st_link);
unsigned long flags;
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_pages.c b/drivers/gpu/drm/i915/gem/i915_gem_pages.c
index f402c2c415c2..75197ca696a8 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_pages.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_pages.c
@@ -8,6 +8,7 @@
#include "i915_gem_object.h"
#include "i915_scatterlist.h"
#include "i915_gem_lmem.h"
+#include "i915_gem_mman.h"
void __i915_gem_object_set_pages(struct drm_i915_gem_object *obj,
struct sg_table *pages,
@@ -207,6 +208,8 @@ int __i915_gem_object_put_pages(struct drm_i915_gem_object *obj)
goto unlock;
}
+ i915_gem_object_release_mmap_offset(obj);
+
/*
* ->put_pages might need to allocate memory for the bit17 swizzle
* array, hence protect them from being reaped by removing them from gtt
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_tiling.c b/drivers/gpu/drm/i915/gem/i915_gem_tiling.c
index 1fa592d82af5..6c7825a2dc2a 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_tiling.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_tiling.c
@@ -11,6 +11,7 @@
#include "i915_drv.h"
#include "i915_gem.h"
#include "i915_gem_ioctls.h"
+#include "i915_gem_mman.h"
#include "i915_gem_object.h"
/**
diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c b/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c
index 9f1a69027a04..63e24f6b0d40 100644
--- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c
+++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c
@@ -564,15 +564,20 @@ static bool assert_mmap_offset(struct drm_i915_private *i915,
int expected)
{
struct drm_i915_gem_object *obj;
+ /* Ownership transferred to parent gem object in create_mmap_offset */
+ struct i915_mmap_offset *mmo = kzalloc(sizeof(*mmo), GFP_KERNEL);
int err;
obj = i915_gem_object_create_internal(i915, size);
if (IS_ERR(obj))
return PTR_ERR(obj);
- err = create_mmap_offset(obj);
+ err = create_mmap_offset(obj, mmo);
+ if (err)
+ kfree(mmo);
i915_gem_object_put(obj);
+
return err == expected;
}
@@ -608,6 +613,8 @@ static int igt_mmap_offset_exhaustion(void *arg)
struct drm_mm *mm = &i915->drm.vma_offset_manager->vm_addr_space_mm;
struct drm_i915_gem_object *obj;
struct drm_mm_node *hole, *next;
+ /* Ownership transferred to parent gem object in create_mmap_offset */
+ struct i915_mmap_offset *mmo = kzalloc(sizeof(*mmo), GFP_KERNEL);
int loop, err;
/* Disable background reaper */
@@ -672,9 +679,10 @@ static int igt_mmap_offset_exhaustion(void *arg)
goto out;
}
- err = create_mmap_offset(obj);
+ err = create_mmap_offset(obj, mmo);
if (err) {
pr_err("Unable to insert object into reclaimed hole\n");
+ kfree(mmo);
goto err_obj;
}
@@ -919,6 +927,15 @@ static int igt_mmap_gtt_revoke(void *arg)
}
GEM_BUG_ON(atomic_read(&obj->bind_count));
+ if (type != I915_MMAP_TYPE_GTT) {
+ __i915_gem_object_put_pages(obj);
+ if (i915_gem_object_has_pages(obj)) {
+ pr_err("Failed to put-pages object!\n");
+ err = -EINVAL;
+ goto out_unmap;
+ }
+ }
+
err = check_absent(addr, obj->base.size);
if (err)
goto out_unmap;
diff --git a/drivers/gpu/drm/i915/gt/intel_reset.c b/drivers/gpu/drm/i915/gt/intel_reset.c
index b7007cd78c6f..777eaa483676 100644
--- a/drivers/gpu/drm/i915/gt/intel_reset.c
+++ b/drivers/gpu/drm/i915/gt/intel_reset.c
@@ -678,8 +678,13 @@ static void revoke_mmaps(struct intel_gt *gt)
continue;
GEM_BUG_ON(vma->fence != >->ggtt->fence_regs[i]);
- node = &vma->obj->base.vma_node;
+
+ if (!vma->mmo)
+ continue;
+
+ node = &vma->mmo->vma_node;
vma_offset = vma->ggtt_view.partial.offset << PAGE_SHIFT;
+
unmap_mapping_range(gt->i915->drm.anon_inode->i_mapping,
drm_vma_node_offset_addr(node) + vma_offset,
vma->size,
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 8d7049792d62..5fe071640893 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -2662,18 +2662,12 @@ const struct dev_pm_ops i915_pm_ops = {
.runtime_resume = intel_runtime_resume,
};
-static const struct vm_operations_struct i915_gem_vm_ops = {
- .fault = i915_gem_fault,
- .open = drm_gem_vm_open,
- .close = drm_gem_vm_close,
-};
-
static const struct file_operations i915_driver_fops = {
.owner = THIS_MODULE,
.open = drm_open,
.release = drm_release,
.unlocked_ioctl = drm_ioctl,
- .mmap = drm_gem_mmap,
+ .mmap = i915_gem_mmap,
.poll = drm_poll,
.read = drm_read,
.compat_ioctl = i915_compat_ioctl,
@@ -2762,7 +2756,6 @@ static struct drm_driver driver = {
.gem_close_object = i915_gem_close_object,
.gem_free_object_unlocked = i915_gem_free_object,
- .gem_vm_ops = &i915_gem_vm_ops,
.prime_handle_to_fd = drm_gem_prime_handle_to_fd,
.prime_fd_to_handle = drm_gem_prime_fd_to_handle,
@@ -2773,7 +2766,6 @@ static struct drm_driver driver = {
.get_scanout_position = i915_get_crtc_scanoutpos,
.dumb_create = i915_gem_dumb_create,
- .dumb_map_offset = i915_gem_mmap_gtt,
.ioctls = i915_ioctls,
.num_ioctls = ARRAY_SIZE(i915_ioctls),
.fops = &i915_driver_fops,
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 1779f600fcfb..a37c77ba5c40 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1861,8 +1861,6 @@ i915_mutex_lock_interruptible(struct drm_device *dev)
int i915_gem_dumb_create(struct drm_file *file_priv,
struct drm_device *dev,
struct drm_mode_create_dumb *args);
-int i915_gem_mmap_gtt(struct drm_file *file_priv, struct drm_device *dev,
- u32 handle, u64 *offset);
int i915_gem_mmap_gtt_version(void);
int __must_check i915_gem_set_global_seqno(struct drm_device *dev, u32 seqno);
@@ -1886,7 +1884,6 @@ void i915_gem_driver_release(struct drm_i915_private *dev_priv);
void i915_gem_suspend(struct drm_i915_private *dev_priv);
void i915_gem_suspend_late(struct drm_i915_private *dev_priv);
void i915_gem_resume(struct drm_i915_private *dev_priv);
-vm_fault_t i915_gem_fault(struct vm_fault *vmf);
int i915_gem_open(struct drm_i915_private *i915, struct drm_file *file);
void i915_gem_release(struct drm_device *dev, struct drm_file *file);
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 43c532756c7c..1b1aa7fc15ba 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -44,6 +44,7 @@
#include "gem/i915_gem_clflush.h"
#include "gem/i915_gem_context.h"
#include "gem/i915_gem_ioctls.h"
+#include "gem/i915_gem_mman.h"
#include "gem/i915_gem_pm.h"
#include "gt/intel_engine_user.h"
#include "gt/intel_gt.h"
@@ -854,7 +855,7 @@ void i915_gem_runtime_suspend(struct drm_i915_private *i915)
list_for_each_entry_safe(obj, on,
&i915->ggtt.userfault_list, userfault_link)
- __i915_gem_object_release_mmap(obj);
+ __i915_gem_object_release_mmap_gtt(obj);
/*
* The fence will be lost when the device powers down. If any were
diff --git a/drivers/gpu/drm/i915/i915_getparam.c b/drivers/gpu/drm/i915/i915_getparam.c
index cf8a8c3ef047..54fce81d5724 100644
--- a/drivers/gpu/drm/i915/i915_getparam.c
+++ b/drivers/gpu/drm/i915/i915_getparam.c
@@ -2,6 +2,7 @@
* SPDX-License-Identifier: MIT
*/
+#include "gem/i915_gem_mman.h"
#include "gt/intel_engine_user.h"
#include "i915_drv.h"
diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c
index e5512f26e20a..5108dcb2e5b1 100644
--- a/drivers/gpu/drm/i915/i915_vma.c
+++ b/drivers/gpu/drm/i915/i915_vma.c
@@ -1054,7 +1054,7 @@ static void __i915_vma_iounmap(struct i915_vma *vma)
void i915_vma_revoke_mmap(struct i915_vma *vma)
{
- struct drm_vma_offset_node *node = &vma->obj->base.vma_node;
+ struct drm_vma_offset_node *node;
u64 vma_offset;
lockdep_assert_held(&vma->vm->mutex);
@@ -1065,6 +1065,7 @@ void i915_vma_revoke_mmap(struct i915_vma *vma)
GEM_BUG_ON(!i915_vma_is_map_and_fenceable(vma));
GEM_BUG_ON(!vma->obj->userfault_count);
+ node = &vma->mmo->vma_node;
vma_offset = vma->ggtt_view.partial.offset << PAGE_SHIFT;
unmap_mapping_range(vma->vm->i915->drm.anon_inode->i_mapping,
drm_vma_node_offset_addr(node) + vma_offset,
diff --git a/drivers/gpu/drm/i915/i915_vma.h b/drivers/gpu/drm/i915/i915_vma.h
index 465932813bc5..f09f4f513c41 100644
--- a/drivers/gpu/drm/i915/i915_vma.h
+++ b/drivers/gpu/drm/i915/i915_vma.h
@@ -63,6 +63,9 @@ struct i915_vma {
u64 display_alignment;
struct i915_page_sizes page_sizes;
+ /* mmap-offset associated with fencing for this vma */
+ struct i915_mmap_offset *mmo;
+
u32 fence_size;
u32 fence_alignment;
--
2.23.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [Intel-gfx] [PATCH 1/4] drm/i915: Allow i915 to manage the vma offset nodes instead of drm core
@ 2019-11-15 11:45 ` Abdiel Janulgue
0 siblings, 0 replies; 28+ messages in thread
From: Abdiel Janulgue @ 2019-11-15 11:45 UTC (permalink / raw)
To: intel-gfx; +Cc: Matthew Auld
Have i915 replace the core drm_gem_mmap implementation to overcome its
limitation in having only a single mmap offset node per gem object.
This change allows us to have multiple mmap offsets per object and
enables a mmapping instance to use unique fault-handlers per user vma.
This allows i915 to store extra data within vma->vm_private_data and
assign the pagefault ops for each mmap instance allowing objects to use
multiple fault handlers depending on its backing storage.
v2:
- Fix race condition exposed by gem_mmap_gtt@close-race. Simplify
lifetime management of the mmap offset objects be ensuring it is
owned by the parent gem object instead of refcounting.
- Track mmo used by fencing to Avoid locking when revoking mmaps
during GPU reset.
- Rebase.
v3:
- Simplify mmo tracking
v4:
- use vma->mmo in __i915_gem_object_release_mmap_gtt
v5:
- Revoke CPU mmaps on i915_gem_object_unbind() since unlike GTT
mmaps, they don't have bound i915_vmas objects. Rebase.
v6: Minor tweaks, header re-org (Chris)
Signed-off-by: Abdiel Janulgue <abdiel.janulgue@linux.intel.com>
Cc: Matthew Auld <matthew.auld@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
---
drivers/gpu/drm/i915/gem/i915_gem_domain.c | 3 +-
drivers/gpu/drm/i915/gem/i915_gem_mman.c | 243 +++++++++++++++---
drivers/gpu/drm/i915/gem/i915_gem_mman.h | 28 ++
drivers/gpu/drm/i915/gem/i915_gem_object.c | 14 +
drivers/gpu/drm/i915/gem/i915_gem_object.h | 7 +-
.../gpu/drm/i915/gem/i915_gem_object_types.h | 19 ++
drivers/gpu/drm/i915/gem/i915_gem_pages.c | 3 +
drivers/gpu/drm/i915/gem/i915_gem_tiling.c | 1 +
.../drm/i915/gem/selftests/i915_gem_mman.c | 21 +-
drivers/gpu/drm/i915/gt/intel_reset.c | 7 +-
drivers/gpu/drm/i915/i915_drv.c | 10 +-
drivers/gpu/drm/i915/i915_drv.h | 3 -
drivers/gpu/drm/i915/i915_gem.c | 3 +-
drivers/gpu/drm/i915/i915_getparam.c | 1 +
drivers/gpu/drm/i915/i915_vma.c | 3 +-
drivers/gpu/drm/i915/i915_vma.h | 3 +
16 files changed, 313 insertions(+), 56 deletions(-)
create mode 100644 drivers/gpu/drm/i915/gem/i915_gem_mman.h
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_domain.c b/drivers/gpu/drm/i915/gem/i915_gem_domain.c
index e2af63af67ad..fd1c6f12b77a 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_domain.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_domain.c
@@ -13,6 +13,7 @@
#include "i915_gem_object.h"
#include "i915_vma.h"
#include "i915_gem_lmem.h"
+#include "i915_gem_mman.h"
static void __i915_gem_object_flush_for_display(struct drm_i915_gem_object *obj)
{
@@ -255,7 +256,7 @@ int i915_gem_object_set_cache_level(struct drm_i915_gem_object *obj,
}
if (obj->userfault_count)
- __i915_gem_object_release_mmap(obj);
+ __i915_gem_object_release_mmap_gtt(obj);
/*
* As we no longer need a fence for GTT access,
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_mman.c b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
index d60973603cc1..e602dfb44532 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_mman.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
@@ -14,6 +14,7 @@
#include "i915_gem_gtt.h"
#include "i915_gem_ioctls.h"
#include "i915_gem_object.h"
+#include "i915_gem_mman.h"
#include "i915_trace.h"
#include "i915_vma.h"
@@ -219,7 +220,8 @@ vm_fault_t i915_gem_fault(struct vm_fault *vmf)
{
#define MIN_CHUNK_PAGES (SZ_1M >> PAGE_SHIFT)
struct vm_area_struct *area = vmf->vma;
- struct drm_i915_gem_object *obj = to_intel_bo(area->vm_private_data);
+ struct i915_mmap_offset *priv = area->vm_private_data;
+ struct drm_i915_gem_object *obj = priv->obj;
struct drm_device *dev = obj->base.dev;
struct drm_i915_private *i915 = to_i915(dev);
struct intel_runtime_pm *rpm = &i915->runtime_pm;
@@ -312,6 +314,9 @@ vm_fault_t i915_gem_fault(struct vm_fault *vmf)
list_add(&obj->userfault_link, &i915->ggtt.userfault_list);
mutex_unlock(&i915->ggtt.vm.mutex);
+ /* Track the mmo associated with the fenced vma */
+ vma->mmo = priv;
+
if (IS_ACTIVE(CONFIG_DRM_I915_USERFAULT_AUTOSUSPEND))
intel_wakeref_auto(&i915->ggtt.userfault_wakeref,
msecs_to_jiffies_timeout(CONFIG_DRM_I915_USERFAULT_AUTOSUSPEND));
@@ -358,28 +363,19 @@ vm_fault_t i915_gem_fault(struct vm_fault *vmf)
}
}
-void __i915_gem_object_release_mmap(struct drm_i915_gem_object *obj)
+void __i915_gem_object_release_mmap_gtt(struct drm_i915_gem_object *obj)
{
struct i915_vma *vma;
GEM_BUG_ON(!obj->userfault_count);
- obj->userfault_count = 0;
- list_del(&obj->userfault_link);
- drm_vma_node_unmap(&obj->base.vma_node,
- obj->base.dev->anon_inode->i_mapping);
-
for_each_ggtt_vma(vma, obj)
- i915_vma_unset_userfault(vma);
+ i915_vma_revoke_mmap(vma);
+
+ GEM_BUG_ON(obj->userfault_count);
}
/**
- * i915_gem_object_release_mmap - remove physical page mappings
- * @obj: obj in question
- *
- * Preserve the reservation of the mmapping with the DRM core code, but
- * relinquish ownership of the pages back to the system.
- *
* It is vital that we remove the page mapping if we have mapped a tiled
* object through the GTT and then lose the fence register due to
* resource pressure. Similarly if the object has been moved out of the
@@ -387,7 +383,7 @@ void __i915_gem_object_release_mmap(struct drm_i915_gem_object *obj)
* mapping will then trigger a page fault on the next user access, allowing
* fixup by i915_gem_fault().
*/
-void i915_gem_object_release_mmap(struct drm_i915_gem_object *obj)
+static void i915_gem_object_release_mmap_gtt(struct drm_i915_gem_object *obj)
{
struct drm_i915_private *i915 = to_i915(obj->base.dev);
intel_wakeref_t wakeref;
@@ -406,7 +402,7 @@ void i915_gem_object_release_mmap(struct drm_i915_gem_object *obj)
if (!obj->userfault_count)
goto out;
- __i915_gem_object_release_mmap(obj);
+ __i915_gem_object_release_mmap_gtt(obj);
/* Ensure that the CPU's PTE are revoked and there are not outstanding
* memory transactions from userspace before we return. The TLB
@@ -422,15 +418,63 @@ void i915_gem_object_release_mmap(struct drm_i915_gem_object *obj)
intel_runtime_pm_put(&i915->runtime_pm, wakeref);
}
-static int create_mmap_offset(struct drm_i915_gem_object *obj)
+void i915_gem_object_release_mmap_offset(struct drm_i915_gem_object *obj)
+{
+ struct i915_mmap_offset *mmo;
+
+ mutex_lock(&obj->mmo_lock);
+ list_for_each_entry(mmo, &obj->mmap_offsets, offset) {
+ /* vma_node_unmap for GTT mmaps handled already in
+ * __i915_gem_object_release_mmap_gtt
+ */
+ if (mmo->mmap_type != I915_MMAP_TYPE_GTT)
+ drm_vma_node_unmap(&mmo->vma_node,
+ obj->base.dev->anon_inode->i_mapping);
+ }
+ mutex_unlock(&obj->mmo_lock);
+}
+
+/**
+ * i915_gem_object_release_mmap - remove physical page mappings
+ * @obj: obj in question
+ *
+ * Preserve the reservation of the mmapping with the DRM core code, but
+ * relinquish ownership of the pages back to the system.
+ */
+void i915_gem_object_release_mmap(struct drm_i915_gem_object *obj)
+{
+ i915_gem_object_release_mmap_gtt(obj);
+ i915_gem_object_release_mmap_offset(obj);
+}
+
+static void init_mmap_offset(struct drm_i915_gem_object *obj,
+ struct i915_mmap_offset *mmo)
+{
+ mutex_lock(&obj->mmo_lock);
+ list_add(&mmo->offset, &obj->mmap_offsets);
+ mutex_unlock(&obj->mmo_lock);
+
+ mmo->obj = obj;
+ mmo->dev = obj->base.dev;
+}
+
+static int create_mmap_offset(struct drm_i915_gem_object *obj,
+ struct i915_mmap_offset *mmo)
{
struct drm_i915_private *i915 = to_i915(obj->base.dev);
struct intel_gt *gt = &i915->gt;
+ struct drm_device *dev = obj->base.dev;
int err;
- err = drm_gem_create_mmap_offset(&obj->base);
- if (likely(!err))
+ drm_vma_node_reset(&mmo->vma_node);
+ if (mmo->file)
+ drm_vma_node_allow(&mmo->vma_node, mmo->file);
+ err = drm_vma_offset_add(dev->vma_offset_manager, &mmo->vma_node,
+ obj->base.size / PAGE_SIZE);
+ if (likely(!err)) {
+ init_mmap_offset(obj, mmo);
return 0;
+ }
/* Attempt to reap some mmap space from dead objects */
err = intel_gt_retire_requests_timeout(gt, MAX_SCHEDULE_TIMEOUT);
@@ -438,34 +482,56 @@ static int create_mmap_offset(struct drm_i915_gem_object *obj)
return err;
i915_gem_drain_freed_objects(i915);
- return drm_gem_create_mmap_offset(&obj->base);
+ err = drm_vma_offset_add(dev->vma_offset_manager, &mmo->vma_node,
+ obj->base.size / PAGE_SIZE);
+ if (err)
+ return err;
+
+ init_mmap_offset(obj, mmo);
+ return 0;
}
-int
-i915_gem_mmap_gtt(struct drm_file *file,
- struct drm_device *dev,
- u32 handle,
- u64 *offset)
+static int
+__assign_gem_object_mmap_data(struct drm_file *file,
+ u32 handle,
+ enum i915_mmap_type mmap_type,
+ u64 *offset)
{
struct drm_i915_gem_object *obj;
+ struct i915_mmap_offset *mmo;
int ret;
- if (!i915_ggtt_has_aperture(&to_i915(dev)->ggtt))
- return -ENODEV;
-
obj = i915_gem_object_lookup(file, handle);
if (!obj)
return -ENOENT;
- if (i915_gem_object_never_bind_ggtt(obj)) {
+ if (mmap_type == I915_MMAP_TYPE_GTT &&
+ i915_gem_object_never_bind_ggtt(obj)) {
ret = -ENODEV;
goto out;
}
- ret = create_mmap_offset(obj);
- if (ret == 0)
- *offset = drm_vma_node_offset_addr(&obj->base.vma_node);
+ if (mmap_type != I915_MMAP_TYPE_GTT &&
+ !i915_gem_object_has_struct_page(obj)) {
+ ret = -ENODEV;
+ goto out;
+ }
+
+ mmo = kzalloc(sizeof(*mmo), GFP_KERNEL);
+ if (!mmo) {
+ ret = -ENOMEM;
+ goto out;
+ }
+
+ mmo->file = file;
+ ret = create_mmap_offset(obj, mmo);
+ if (ret) {
+ kfree(mmo);
+ goto out;
+ }
+ mmo->mmap_type = mmap_type;
+ *offset = drm_vma_node_offset_addr(&mmo->vma_node);
out:
i915_gem_object_put(obj);
return ret;
@@ -492,7 +558,118 @@ i915_gem_mmap_gtt_ioctl(struct drm_device *dev, void *data,
{
struct drm_i915_gem_mmap_gtt *args = data;
- return i915_gem_mmap_gtt(file, dev, args->handle, &args->offset);
+ return __assign_gem_object_mmap_data(file, args->handle,
+ I915_MMAP_TYPE_GTT,
+ &args->offset);
+}
+
+void i915_mmap_offset_destroy(struct i915_mmap_offset *mmo, struct mutex *mutex)
+{
+ if (mmo->file)
+ drm_vma_node_revoke(&mmo->vma_node, mmo->file);
+ drm_vma_offset_remove(mmo->dev->vma_offset_manager, &mmo->vma_node);
+
+ mutex_lock(mutex);
+ list_del(&mmo->offset);
+ mutex_unlock(mutex);
+
+ kfree(mmo);
+}
+
+static void i915_gem_vm_open(struct vm_area_struct *vma)
+{
+ struct i915_mmap_offset *mmo = vma->vm_private_data;
+ struct drm_i915_gem_object *obj = mmo->obj;
+
+ GEM_BUG_ON(!obj);
+ i915_gem_object_get(obj);
+}
+
+static void i915_gem_vm_close(struct vm_area_struct *vma)
+{
+ struct i915_mmap_offset *mmo = vma->vm_private_data;
+ struct drm_i915_gem_object *obj = mmo->obj;
+
+ GEM_BUG_ON(!obj);
+ i915_gem_object_put(obj);
+}
+
+static const struct vm_operations_struct i915_gem_gtt_vm_ops = {
+ .fault = i915_gem_fault,
+ .open = i915_gem_vm_open,
+ .close = i915_gem_vm_close,
+};
+
+/* This overcomes the limitation in drm_gem_mmap's assignment of a
+ * drm_gem_object as the vma->vm_private_data. Since we need to
+ * be able to resolve multiple mmap offsets which could be tied
+ * to a single gem object.
+ */
+int i915_gem_mmap(struct file *filp, struct vm_area_struct *vma)
+{
+ struct drm_vma_offset_node *node;
+ struct drm_file *priv = filp->private_data;
+ struct drm_device *dev = priv->minor->dev;
+ struct i915_mmap_offset *mmo = NULL;
+ struct drm_gem_object *obj = NULL;
+
+ if (drm_dev_is_unplugged(dev))
+ return -ENODEV;
+
+ drm_vma_offset_lock_lookup(dev->vma_offset_manager);
+ node = drm_vma_offset_exact_lookup_locked(dev->vma_offset_manager,
+ vma->vm_pgoff,
+ vma_pages(vma));
+ if (likely(node)) {
+ mmo = container_of(node, struct i915_mmap_offset,
+ vma_node);
+ /*
+ * In our dependency chain, the drm_vma_offset_node
+ * depends on the validity of the mmo, which depends on
+ * the gem object. However the only reference we have
+ * at this point is the mmo (as the parent of the node).
+ * Try to check if the gem object was at least cleared.
+ */
+ if (!mmo || !mmo->obj) {
+ drm_vma_offset_unlock_lookup(dev->vma_offset_manager);
+ return -EINVAL;
+ }
+ /*
+ * Skip 0-refcnted objects as it is in the process of being
+ * destroyed and will be invalid when the vma manager lock
+ * is released.
+ */
+ obj = &mmo->obj->base;
+ if (!kref_get_unless_zero(&obj->refcount))
+ obj = NULL;
+
+ }
+ drm_vma_offset_unlock_lookup(dev->vma_offset_manager);
+
+ if (!obj)
+ return -EINVAL;
+
+ if (!drm_vma_node_is_allowed(node, priv)) {
+ drm_gem_object_put_unlocked(obj);
+ return -EACCES;
+ }
+
+ if (to_intel_bo(obj)->readonly) {
+ if (vma->vm_flags & VM_WRITE) {
+ drm_gem_object_put_unlocked(obj);
+ return -EINVAL;
+ }
+ vma->vm_flags &= ~VM_MAYWRITE;
+ }
+
+ vma->vm_flags |= VM_PFNMAP | VM_DONTEXPAND | VM_DONTDUMP;
+ vma->vm_page_prot = pgprot_writecombine(vm_get_page_prot(vma->vm_flags));
+ vma->vm_page_prot = pgprot_decrypted(vma->vm_page_prot);
+ vma->vm_private_data = mmo;
+
+ vma->vm_ops = &i915_gem_gtt_vm_ops;
+
+ return 0;
}
#if IS_ENABLED(CONFIG_DRM_I915_SELFTEST)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_mman.h b/drivers/gpu/drm/i915/gem/i915_gem_mman.h
new file mode 100644
index 000000000000..25a3c4d6cd65
--- /dev/null
+++ b/drivers/gpu/drm/i915/gem/i915_gem_mman.h
@@ -0,0 +1,28 @@
+/*
+ * SPDX-License-Identifier: MIT
+ *
+ * Copyright © 2019 Intel Corporation
+ */
+
+#ifndef __I915_GEM_MMAN_H__
+#define __I915_GEM_MMAN_H__
+
+#include <linux/mm_types.h>
+#include <linux/types.h>
+
+struct drm_device;
+struct drm_file;
+struct drm_i915_gem_object;
+struct file;
+struct i915_mmap_offset;
+struct mutex;
+
+int i915_gem_mmap(struct file *filp, struct vm_area_struct *vma);
+vm_fault_t i915_gem_fault(struct vm_fault *vmf);
+void i915_mmap_offset_destroy(struct i915_mmap_offset *mmo, struct mutex *mutex);
+
+void __i915_gem_object_release_mmap_gtt(struct drm_i915_gem_object *obj);
+void i915_gem_object_release_mmap(struct drm_i915_gem_object *obj);
+void i915_gem_object_release_mmap_offset(struct drm_i915_gem_object *obj);
+
+#endif
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.c b/drivers/gpu/drm/i915/gem/i915_gem_object.c
index db103d3c8760..c4f4f10c852d 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_object.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_object.c
@@ -29,6 +29,7 @@
#include "i915_drv.h"
#include "i915_gem_clflush.h"
#include "i915_gem_context.h"
+#include "i915_gem_mman.h"
#include "i915_gem_object.h"
#include "i915_globals.h"
#include "i915_trace.h"
@@ -61,6 +62,9 @@ void i915_gem_object_init(struct drm_i915_gem_object *obj,
INIT_LIST_HEAD(&obj->lut_list);
+ mutex_init(&obj->mmo_lock);
+ INIT_LIST_HEAD(&obj->mmap_offsets);
+
init_rcu_head(&obj->rcu);
obj->ops = ops;
@@ -158,6 +162,8 @@ static void __i915_gem_free_objects(struct drm_i915_private *i915,
wakeref = intel_runtime_pm_get(&i915->runtime_pm);
llist_for_each_entry_safe(obj, on, freed, freed) {
+ struct i915_mmap_offset *mmo, *mn;
+
trace_i915_gem_object_destroy(obj);
if (!list_empty(&obj->vma.list)) {
@@ -183,6 +189,14 @@ static void __i915_gem_free_objects(struct drm_i915_private *i915,
spin_unlock(&obj->vma.lock);
}
+ i915_gem_object_release_mmap(obj);
+
+ list_for_each_entry_safe(mmo, mn, &obj->mmap_offsets, offset) {
+ mmo->obj = NULL;
+ i915_mmap_offset_destroy(mmo, &obj->mmo_lock);
+ }
+
+ GEM_BUG_ON(!list_empty(&obj->mmap_offsets));
GEM_BUG_ON(atomic_read(&obj->bind_count));
GEM_BUG_ON(obj->userfault_count);
GEM_BUG_ON(!list_empty(&obj->lut_list));
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.h b/drivers/gpu/drm/i915/gem/i915_gem_object.h
index e5750d506cc9..9b61f18606e0 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_object.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_object.h
@@ -132,13 +132,13 @@ void i915_gem_object_unlock_fence(struct drm_i915_gem_object *obj,
static inline void
i915_gem_object_set_readonly(struct drm_i915_gem_object *obj)
{
- obj->base.vma_node.readonly = true;
+ obj->readonly = true;
}
static inline bool
i915_gem_object_is_readonly(const struct drm_i915_gem_object *obj)
{
- return obj->base.vma_node.readonly;
+ return obj->readonly;
}
static inline bool
@@ -387,9 +387,6 @@ static inline void i915_gem_object_unpin_map(struct drm_i915_gem_object *obj)
i915_gem_object_unpin_pages(obj);
}
-void __i915_gem_object_release_mmap(struct drm_i915_gem_object *obj);
-void i915_gem_object_release_mmap(struct drm_i915_gem_object *obj);
-
void
i915_gem_object_flush_write_domain(struct drm_i915_gem_object *obj,
unsigned int flush_domains);
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object_types.h b/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
index 15f8297dc34e..632d6b804cfb 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
@@ -63,6 +63,20 @@ struct drm_i915_gem_object_ops {
void (*release)(struct drm_i915_gem_object *obj);
};
+enum i915_mmap_type {
+ I915_MMAP_TYPE_GTT = 0,
+};
+
+struct i915_mmap_offset {
+ struct drm_device *dev;
+ struct drm_vma_offset_node vma_node;
+ struct drm_i915_gem_object *obj;
+ struct drm_file *file;
+ enum i915_mmap_type mmap_type;
+
+ struct list_head offset;
+};
+
struct drm_i915_gem_object {
struct drm_gem_object base;
@@ -118,6 +132,11 @@ struct drm_i915_gem_object {
unsigned int userfault_count;
struct list_head userfault_link;
+ /* Protects access to mmap offsets */
+ struct mutex mmo_lock;
+ struct list_head mmap_offsets;
+ bool readonly:1;
+
I915_SELFTEST_DECLARE(struct list_head st_link);
unsigned long flags;
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_pages.c b/drivers/gpu/drm/i915/gem/i915_gem_pages.c
index f402c2c415c2..75197ca696a8 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_pages.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_pages.c
@@ -8,6 +8,7 @@
#include "i915_gem_object.h"
#include "i915_scatterlist.h"
#include "i915_gem_lmem.h"
+#include "i915_gem_mman.h"
void __i915_gem_object_set_pages(struct drm_i915_gem_object *obj,
struct sg_table *pages,
@@ -207,6 +208,8 @@ int __i915_gem_object_put_pages(struct drm_i915_gem_object *obj)
goto unlock;
}
+ i915_gem_object_release_mmap_offset(obj);
+
/*
* ->put_pages might need to allocate memory for the bit17 swizzle
* array, hence protect them from being reaped by removing them from gtt
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_tiling.c b/drivers/gpu/drm/i915/gem/i915_gem_tiling.c
index 1fa592d82af5..6c7825a2dc2a 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_tiling.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_tiling.c
@@ -11,6 +11,7 @@
#include "i915_drv.h"
#include "i915_gem.h"
#include "i915_gem_ioctls.h"
+#include "i915_gem_mman.h"
#include "i915_gem_object.h"
/**
diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c b/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c
index 9f1a69027a04..63e24f6b0d40 100644
--- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c
+++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c
@@ -564,15 +564,20 @@ static bool assert_mmap_offset(struct drm_i915_private *i915,
int expected)
{
struct drm_i915_gem_object *obj;
+ /* Ownership transferred to parent gem object in create_mmap_offset */
+ struct i915_mmap_offset *mmo = kzalloc(sizeof(*mmo), GFP_KERNEL);
int err;
obj = i915_gem_object_create_internal(i915, size);
if (IS_ERR(obj))
return PTR_ERR(obj);
- err = create_mmap_offset(obj);
+ err = create_mmap_offset(obj, mmo);
+ if (err)
+ kfree(mmo);
i915_gem_object_put(obj);
+
return err == expected;
}
@@ -608,6 +613,8 @@ static int igt_mmap_offset_exhaustion(void *arg)
struct drm_mm *mm = &i915->drm.vma_offset_manager->vm_addr_space_mm;
struct drm_i915_gem_object *obj;
struct drm_mm_node *hole, *next;
+ /* Ownership transferred to parent gem object in create_mmap_offset */
+ struct i915_mmap_offset *mmo = kzalloc(sizeof(*mmo), GFP_KERNEL);
int loop, err;
/* Disable background reaper */
@@ -672,9 +679,10 @@ static int igt_mmap_offset_exhaustion(void *arg)
goto out;
}
- err = create_mmap_offset(obj);
+ err = create_mmap_offset(obj, mmo);
if (err) {
pr_err("Unable to insert object into reclaimed hole\n");
+ kfree(mmo);
goto err_obj;
}
@@ -919,6 +927,15 @@ static int igt_mmap_gtt_revoke(void *arg)
}
GEM_BUG_ON(atomic_read(&obj->bind_count));
+ if (type != I915_MMAP_TYPE_GTT) {
+ __i915_gem_object_put_pages(obj);
+ if (i915_gem_object_has_pages(obj)) {
+ pr_err("Failed to put-pages object!\n");
+ err = -EINVAL;
+ goto out_unmap;
+ }
+ }
+
err = check_absent(addr, obj->base.size);
if (err)
goto out_unmap;
diff --git a/drivers/gpu/drm/i915/gt/intel_reset.c b/drivers/gpu/drm/i915/gt/intel_reset.c
index b7007cd78c6f..777eaa483676 100644
--- a/drivers/gpu/drm/i915/gt/intel_reset.c
+++ b/drivers/gpu/drm/i915/gt/intel_reset.c
@@ -678,8 +678,13 @@ static void revoke_mmaps(struct intel_gt *gt)
continue;
GEM_BUG_ON(vma->fence != >->ggtt->fence_regs[i]);
- node = &vma->obj->base.vma_node;
+
+ if (!vma->mmo)
+ continue;
+
+ node = &vma->mmo->vma_node;
vma_offset = vma->ggtt_view.partial.offset << PAGE_SHIFT;
+
unmap_mapping_range(gt->i915->drm.anon_inode->i_mapping,
drm_vma_node_offset_addr(node) + vma_offset,
vma->size,
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 8d7049792d62..5fe071640893 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -2662,18 +2662,12 @@ const struct dev_pm_ops i915_pm_ops = {
.runtime_resume = intel_runtime_resume,
};
-static const struct vm_operations_struct i915_gem_vm_ops = {
- .fault = i915_gem_fault,
- .open = drm_gem_vm_open,
- .close = drm_gem_vm_close,
-};
-
static const struct file_operations i915_driver_fops = {
.owner = THIS_MODULE,
.open = drm_open,
.release = drm_release,
.unlocked_ioctl = drm_ioctl,
- .mmap = drm_gem_mmap,
+ .mmap = i915_gem_mmap,
.poll = drm_poll,
.read = drm_read,
.compat_ioctl = i915_compat_ioctl,
@@ -2762,7 +2756,6 @@ static struct drm_driver driver = {
.gem_close_object = i915_gem_close_object,
.gem_free_object_unlocked = i915_gem_free_object,
- .gem_vm_ops = &i915_gem_vm_ops,
.prime_handle_to_fd = drm_gem_prime_handle_to_fd,
.prime_fd_to_handle = drm_gem_prime_fd_to_handle,
@@ -2773,7 +2766,6 @@ static struct drm_driver driver = {
.get_scanout_position = i915_get_crtc_scanoutpos,
.dumb_create = i915_gem_dumb_create,
- .dumb_map_offset = i915_gem_mmap_gtt,
.ioctls = i915_ioctls,
.num_ioctls = ARRAY_SIZE(i915_ioctls),
.fops = &i915_driver_fops,
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 1779f600fcfb..a37c77ba5c40 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1861,8 +1861,6 @@ i915_mutex_lock_interruptible(struct drm_device *dev)
int i915_gem_dumb_create(struct drm_file *file_priv,
struct drm_device *dev,
struct drm_mode_create_dumb *args);
-int i915_gem_mmap_gtt(struct drm_file *file_priv, struct drm_device *dev,
- u32 handle, u64 *offset);
int i915_gem_mmap_gtt_version(void);
int __must_check i915_gem_set_global_seqno(struct drm_device *dev, u32 seqno);
@@ -1886,7 +1884,6 @@ void i915_gem_driver_release(struct drm_i915_private *dev_priv);
void i915_gem_suspend(struct drm_i915_private *dev_priv);
void i915_gem_suspend_late(struct drm_i915_private *dev_priv);
void i915_gem_resume(struct drm_i915_private *dev_priv);
-vm_fault_t i915_gem_fault(struct vm_fault *vmf);
int i915_gem_open(struct drm_i915_private *i915, struct drm_file *file);
void i915_gem_release(struct drm_device *dev, struct drm_file *file);
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 43c532756c7c..1b1aa7fc15ba 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -44,6 +44,7 @@
#include "gem/i915_gem_clflush.h"
#include "gem/i915_gem_context.h"
#include "gem/i915_gem_ioctls.h"
+#include "gem/i915_gem_mman.h"
#include "gem/i915_gem_pm.h"
#include "gt/intel_engine_user.h"
#include "gt/intel_gt.h"
@@ -854,7 +855,7 @@ void i915_gem_runtime_suspend(struct drm_i915_private *i915)
list_for_each_entry_safe(obj, on,
&i915->ggtt.userfault_list, userfault_link)
- __i915_gem_object_release_mmap(obj);
+ __i915_gem_object_release_mmap_gtt(obj);
/*
* The fence will be lost when the device powers down. If any were
diff --git a/drivers/gpu/drm/i915/i915_getparam.c b/drivers/gpu/drm/i915/i915_getparam.c
index cf8a8c3ef047..54fce81d5724 100644
--- a/drivers/gpu/drm/i915/i915_getparam.c
+++ b/drivers/gpu/drm/i915/i915_getparam.c
@@ -2,6 +2,7 @@
* SPDX-License-Identifier: MIT
*/
+#include "gem/i915_gem_mman.h"
#include "gt/intel_engine_user.h"
#include "i915_drv.h"
diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c
index e5512f26e20a..5108dcb2e5b1 100644
--- a/drivers/gpu/drm/i915/i915_vma.c
+++ b/drivers/gpu/drm/i915/i915_vma.c
@@ -1054,7 +1054,7 @@ static void __i915_vma_iounmap(struct i915_vma *vma)
void i915_vma_revoke_mmap(struct i915_vma *vma)
{
- struct drm_vma_offset_node *node = &vma->obj->base.vma_node;
+ struct drm_vma_offset_node *node;
u64 vma_offset;
lockdep_assert_held(&vma->vm->mutex);
@@ -1065,6 +1065,7 @@ void i915_vma_revoke_mmap(struct i915_vma *vma)
GEM_BUG_ON(!i915_vma_is_map_and_fenceable(vma));
GEM_BUG_ON(!vma->obj->userfault_count);
+ node = &vma->mmo->vma_node;
vma_offset = vma->ggtt_view.partial.offset << PAGE_SHIFT;
unmap_mapping_range(vma->vm->i915->drm.anon_inode->i_mapping,
drm_vma_node_offset_addr(node) + vma_offset,
diff --git a/drivers/gpu/drm/i915/i915_vma.h b/drivers/gpu/drm/i915/i915_vma.h
index 465932813bc5..f09f4f513c41 100644
--- a/drivers/gpu/drm/i915/i915_vma.h
+++ b/drivers/gpu/drm/i915/i915_vma.h
@@ -63,6 +63,9 @@ struct i915_vma {
u64 display_alignment;
struct i915_page_sizes page_sizes;
+ /* mmap-offset associated with fencing for this vma */
+ struct i915_mmap_offset *mmo;
+
u32 fence_size;
u32 fence_alignment;
--
2.23.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 2/4] drm/i915: Introduce DRM_I915_GEM_MMAP_OFFSET
@ 2019-11-15 11:45 ` Abdiel Janulgue
0 siblings, 0 replies; 28+ messages in thread
From: Abdiel Janulgue @ 2019-11-15 11:45 UTC (permalink / raw)
To: intel-gfx; +Cc: Matthew Auld
This is really just an alias of mmap_gtt. The 'mmap offset' nomenclature
comes from the value returned by this ioctl which is the offset into the
device fd which userpace uses with mmap(2).
mmap_gtt was our initial mmap_offset implementation, this extends
our CPU mmap support to allow additional fault handlers that depends on
the object's backing pages.
Note that we multiplex mmap_gtt and mmap_offset through the same ioctl,
and use the zero extending behaviour of drm to differentiate between
them, when we inspect the flags.
v2:
- Drop the alias, just rename the struct (Chris)
- Don't bail out on no PAT when doing WB mmaps
- Prepare uAPI for further extensions
v3:
- drop MMAP_OFFSET_FLAGS
v4:
- Tweaks, header re-org
Signed-off-by: Abdiel Janulgue <abdiel.janulgue@linux.intel.com>
Signed-off-by: Matthew Auld <matthew.auld@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
drivers/gpu/drm/i915/gem/i915_gem_ioctls.h | 4 +-
drivers/gpu/drm/i915/gem/i915_gem_mman.c | 42 ++++++++++++++++---
drivers/gpu/drm/i915/gem/i915_gem_mman.h | 1 +
.../gpu/drm/i915/gem/i915_gem_object_types.h | 3 ++
drivers/gpu/drm/i915/i915_drv.c | 3 +-
drivers/gpu/drm/i915/i915_drv.h | 1 -
include/uapi/drm/i915_drm.h | 27 ++++++++++++
7 files changed, 72 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ioctls.h b/drivers/gpu/drm/i915/gem/i915_gem_ioctls.h
index ddc7f2a52b3e..87d8b27f426d 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_ioctls.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_ioctls.h
@@ -28,8 +28,8 @@ int i915_gem_madvise_ioctl(struct drm_device *dev, void *data,
struct drm_file *file);
int i915_gem_mmap_ioctl(struct drm_device *dev, void *data,
struct drm_file *file);
-int i915_gem_mmap_gtt_ioctl(struct drm_device *dev, void *data,
- struct drm_file *file);
+int i915_gem_mmap_offset_ioctl(struct drm_device *dev, void *data,
+ struct drm_file *file);
int i915_gem_pread_ioctl(struct drm_device *dev, void *data,
struct drm_file *file);
int i915_gem_pwrite_ioctl(struct drm_device *dev, void *data,
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_mman.c b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
index e602dfb44532..d2ed8a463672 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_mman.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
@@ -145,6 +145,9 @@ static unsigned int tile_row_pages(const struct drm_i915_gem_object *obj)
* 3 - Remove implicit set-domain(GTT) and synchronisation on initial
* pagefault; swapin remains transparent.
*
+ * 4 - Support multiple fault handlers per object depending on object's
+ * backing storage (a.k.a. MMAP_OFFSET).
+ *
* Restrictions:
*
* * snoopable objects cannot be accessed via the GTT. It can cause machine
@@ -172,7 +175,7 @@ static unsigned int tile_row_pages(const struct drm_i915_gem_object *obj)
*/
int i915_gem_mmap_gtt_version(void)
{
- return 3;
+ return 4;
}
static inline struct i915_ggtt_view
@@ -538,7 +541,7 @@ __assign_gem_object_mmap_data(struct drm_file *file,
}
/**
- * i915_gem_mmap_gtt_ioctl - prepare an object for GTT mmap'ing
+ * i915_gem_mmap_offset_ioctl - prepare an object for GTT mmap'ing
* @dev: DRM device
* @data: GTT mapping ioctl data
* @file: GEM object info
@@ -553,10 +556,39 @@ __assign_gem_object_mmap_data(struct drm_file *file,
* userspace.
*/
int
-i915_gem_mmap_gtt_ioctl(struct drm_device *dev, void *data,
- struct drm_file *file)
+i915_gem_mmap_offset_ioctl(struct drm_device *dev, void *data,
+ struct drm_file *file)
{
- struct drm_i915_gem_mmap_gtt *args = data;
+ struct drm_i915_private *i915 = to_i915(dev);
+ struct drm_i915_gem_mmap_offset *args = data;
+ enum i915_mmap_type type;
+
+ switch (args->flags) {
+ case I915_MMAP_OFFSET_GTT:
+ if (!i915_ggtt_has_aperture(&i915->ggtt))
+ return -ENODEV;
+ type = I915_MMAP_TYPE_GTT;
+ break;
+
+ case I915_MMAP_OFFSET_WC:
+ if (!boot_cpu_has(X86_FEATURE_PAT))
+ return -ENODEV;
+ type = I915_MMAP_TYPE_WC;
+ break;
+
+ case I915_MMAP_OFFSET_WB:
+ type = I915_MMAP_TYPE_WB;
+ break;
+
+ case I915_MMAP_OFFSET_UC:
+ if (!boot_cpu_has(X86_FEATURE_PAT))
+ return -ENODEV;
+ type = I915_MMAP_TYPE_UC;
+ break;
+
+ default:
+ return -EINVAL;
+ }
return __assign_gem_object_mmap_data(file, args->handle,
I915_MMAP_TYPE_GTT,
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_mman.h b/drivers/gpu/drm/i915/gem/i915_gem_mman.h
index 25a3c4d6cd65..4d3b493e853a 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_mman.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_mman.h
@@ -24,5 +24,6 @@ void i915_mmap_offset_destroy(struct i915_mmap_offset *mmo, struct mutex *mutex)
void __i915_gem_object_release_mmap_gtt(struct drm_i915_gem_object *obj);
void i915_gem_object_release_mmap(struct drm_i915_gem_object *obj);
void i915_gem_object_release_mmap_offset(struct drm_i915_gem_object *obj);
+int i915_gem_mmap_gtt_version(void);
#endif
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object_types.h b/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
index 632d6b804cfb..07237fbfdaac 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
@@ -65,6 +65,9 @@ struct drm_i915_gem_object_ops {
enum i915_mmap_type {
I915_MMAP_TYPE_GTT = 0,
+ I915_MMAP_TYPE_WC,
+ I915_MMAP_TYPE_WB,
+ I915_MMAP_TYPE_UC,
};
struct i915_mmap_offset {
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 5fe071640893..ac6d4470ce75 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -61,6 +61,7 @@
#include "gem/i915_gem_context.h"
#include "gem/i915_gem_ioctls.h"
+#include "gem/i915_gem_mman.h"
#include "gt/intel_gt.h"
#include "gt/intel_gt_pm.h"
#include "gt/intel_rc6.h"
@@ -2714,7 +2715,7 @@ static const struct drm_ioctl_desc i915_ioctls[] = {
DRM_IOCTL_DEF_DRV(I915_GEM_PREAD, i915_gem_pread_ioctl, DRM_RENDER_ALLOW),
DRM_IOCTL_DEF_DRV(I915_GEM_PWRITE, i915_gem_pwrite_ioctl, DRM_RENDER_ALLOW),
DRM_IOCTL_DEF_DRV(I915_GEM_MMAP, i915_gem_mmap_ioctl, DRM_RENDER_ALLOW),
- DRM_IOCTL_DEF_DRV(I915_GEM_MMAP_GTT, i915_gem_mmap_gtt_ioctl, DRM_RENDER_ALLOW),
+ DRM_IOCTL_DEF_DRV(I915_GEM_MMAP_OFFSET, i915_gem_mmap_offset_ioctl, DRM_RENDER_ALLOW),
DRM_IOCTL_DEF_DRV(I915_GEM_SET_DOMAIN, i915_gem_set_domain_ioctl, DRM_RENDER_ALLOW),
DRM_IOCTL_DEF_DRV(I915_GEM_SW_FINISH, i915_gem_sw_finish_ioctl, DRM_RENDER_ALLOW),
DRM_IOCTL_DEF_DRV(I915_GEM_SET_TILING, i915_gem_set_tiling_ioctl, DRM_RENDER_ALLOW),
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index a37c77ba5c40..6ee4b86ca758 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1861,7 +1861,6 @@ i915_mutex_lock_interruptible(struct drm_device *dev)
int i915_gem_dumb_create(struct drm_file *file_priv,
struct drm_device *dev,
struct drm_mode_create_dumb *args);
-int i915_gem_mmap_gtt_version(void);
int __must_check i915_gem_set_global_seqno(struct drm_device *dev, u32 seqno);
diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index 5400d7e057f1..e844c3a8d197 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -395,6 +395,7 @@ typedef struct _drm_i915_sarea {
#define DRM_IOCTL_I915_GEM_PWRITE DRM_IOW (DRM_COMMAND_BASE + DRM_I915_GEM_PWRITE, struct drm_i915_gem_pwrite)
#define DRM_IOCTL_I915_GEM_MMAP DRM_IOWR(DRM_COMMAND_BASE + DRM_I915_GEM_MMAP, struct drm_i915_gem_mmap)
#define DRM_IOCTL_I915_GEM_MMAP_GTT DRM_IOWR(DRM_COMMAND_BASE + DRM_I915_GEM_MMAP_GTT, struct drm_i915_gem_mmap_gtt)
+#define DRM_IOCTL_I915_GEM_MMAP_OFFSET DRM_IOWR(DRM_COMMAND_BASE + DRM_I915_GEM_MMAP_GTT, struct drm_i915_gem_mmap_offset)
#define DRM_IOCTL_I915_GEM_SET_DOMAIN DRM_IOW (DRM_COMMAND_BASE + DRM_I915_GEM_SET_DOMAIN, struct drm_i915_gem_set_domain)
#define DRM_IOCTL_I915_GEM_SW_FINISH DRM_IOW (DRM_COMMAND_BASE + DRM_I915_GEM_SW_FINISH, struct drm_i915_gem_sw_finish)
#define DRM_IOCTL_I915_GEM_SET_TILING DRM_IOWR (DRM_COMMAND_BASE + DRM_I915_GEM_SET_TILING, struct drm_i915_gem_set_tiling)
@@ -793,6 +794,32 @@ struct drm_i915_gem_mmap_gtt {
__u64 offset;
};
+struct drm_i915_gem_mmap_offset {
+ /** Handle for the object being mapped. */
+ __u32 handle;
+ __u32 pad;
+ /**
+ * Fake offset to use for subsequent mmap call
+ *
+ * This is a fixed-size type for 32/64 compatibility.
+ */
+ __u64 offset;
+
+ /**
+ * Flags for extended behaviour.
+ *
+ * It is mandatory that either one of the MMAP_OFFSET flags
+ * should be passed here.
+ */
+ __u64 flags;
+#define I915_MMAP_OFFSET_GTT 0
+#define I915_MMAP_OFFSET_WC 1
+#define I915_MMAP_OFFSET_WB 2
+#define I915_MMAP_OFFSET_UC 3
+
+ __u64 extensions;
+};
+
struct drm_i915_gem_set_domain {
/** Handle for the object */
__u32 handle;
--
2.23.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [Intel-gfx] [PATCH 2/4] drm/i915: Introduce DRM_I915_GEM_MMAP_OFFSET
@ 2019-11-15 11:45 ` Abdiel Janulgue
0 siblings, 0 replies; 28+ messages in thread
From: Abdiel Janulgue @ 2019-11-15 11:45 UTC (permalink / raw)
To: intel-gfx; +Cc: Matthew Auld
This is really just an alias of mmap_gtt. The 'mmap offset' nomenclature
comes from the value returned by this ioctl which is the offset into the
device fd which userpace uses with mmap(2).
mmap_gtt was our initial mmap_offset implementation, this extends
our CPU mmap support to allow additional fault handlers that depends on
the object's backing pages.
Note that we multiplex mmap_gtt and mmap_offset through the same ioctl,
and use the zero extending behaviour of drm to differentiate between
them, when we inspect the flags.
v2:
- Drop the alias, just rename the struct (Chris)
- Don't bail out on no PAT when doing WB mmaps
- Prepare uAPI for further extensions
v3:
- drop MMAP_OFFSET_FLAGS
v4:
- Tweaks, header re-org
Signed-off-by: Abdiel Janulgue <abdiel.janulgue@linux.intel.com>
Signed-off-by: Matthew Auld <matthew.auld@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
drivers/gpu/drm/i915/gem/i915_gem_ioctls.h | 4 +-
drivers/gpu/drm/i915/gem/i915_gem_mman.c | 42 ++++++++++++++++---
drivers/gpu/drm/i915/gem/i915_gem_mman.h | 1 +
.../gpu/drm/i915/gem/i915_gem_object_types.h | 3 ++
drivers/gpu/drm/i915/i915_drv.c | 3 +-
drivers/gpu/drm/i915/i915_drv.h | 1 -
include/uapi/drm/i915_drm.h | 27 ++++++++++++
7 files changed, 72 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ioctls.h b/drivers/gpu/drm/i915/gem/i915_gem_ioctls.h
index ddc7f2a52b3e..87d8b27f426d 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_ioctls.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_ioctls.h
@@ -28,8 +28,8 @@ int i915_gem_madvise_ioctl(struct drm_device *dev, void *data,
struct drm_file *file);
int i915_gem_mmap_ioctl(struct drm_device *dev, void *data,
struct drm_file *file);
-int i915_gem_mmap_gtt_ioctl(struct drm_device *dev, void *data,
- struct drm_file *file);
+int i915_gem_mmap_offset_ioctl(struct drm_device *dev, void *data,
+ struct drm_file *file);
int i915_gem_pread_ioctl(struct drm_device *dev, void *data,
struct drm_file *file);
int i915_gem_pwrite_ioctl(struct drm_device *dev, void *data,
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_mman.c b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
index e602dfb44532..d2ed8a463672 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_mman.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
@@ -145,6 +145,9 @@ static unsigned int tile_row_pages(const struct drm_i915_gem_object *obj)
* 3 - Remove implicit set-domain(GTT) and synchronisation on initial
* pagefault; swapin remains transparent.
*
+ * 4 - Support multiple fault handlers per object depending on object's
+ * backing storage (a.k.a. MMAP_OFFSET).
+ *
* Restrictions:
*
* * snoopable objects cannot be accessed via the GTT. It can cause machine
@@ -172,7 +175,7 @@ static unsigned int tile_row_pages(const struct drm_i915_gem_object *obj)
*/
int i915_gem_mmap_gtt_version(void)
{
- return 3;
+ return 4;
}
static inline struct i915_ggtt_view
@@ -538,7 +541,7 @@ __assign_gem_object_mmap_data(struct drm_file *file,
}
/**
- * i915_gem_mmap_gtt_ioctl - prepare an object for GTT mmap'ing
+ * i915_gem_mmap_offset_ioctl - prepare an object for GTT mmap'ing
* @dev: DRM device
* @data: GTT mapping ioctl data
* @file: GEM object info
@@ -553,10 +556,39 @@ __assign_gem_object_mmap_data(struct drm_file *file,
* userspace.
*/
int
-i915_gem_mmap_gtt_ioctl(struct drm_device *dev, void *data,
- struct drm_file *file)
+i915_gem_mmap_offset_ioctl(struct drm_device *dev, void *data,
+ struct drm_file *file)
{
- struct drm_i915_gem_mmap_gtt *args = data;
+ struct drm_i915_private *i915 = to_i915(dev);
+ struct drm_i915_gem_mmap_offset *args = data;
+ enum i915_mmap_type type;
+
+ switch (args->flags) {
+ case I915_MMAP_OFFSET_GTT:
+ if (!i915_ggtt_has_aperture(&i915->ggtt))
+ return -ENODEV;
+ type = I915_MMAP_TYPE_GTT;
+ break;
+
+ case I915_MMAP_OFFSET_WC:
+ if (!boot_cpu_has(X86_FEATURE_PAT))
+ return -ENODEV;
+ type = I915_MMAP_TYPE_WC;
+ break;
+
+ case I915_MMAP_OFFSET_WB:
+ type = I915_MMAP_TYPE_WB;
+ break;
+
+ case I915_MMAP_OFFSET_UC:
+ if (!boot_cpu_has(X86_FEATURE_PAT))
+ return -ENODEV;
+ type = I915_MMAP_TYPE_UC;
+ break;
+
+ default:
+ return -EINVAL;
+ }
return __assign_gem_object_mmap_data(file, args->handle,
I915_MMAP_TYPE_GTT,
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_mman.h b/drivers/gpu/drm/i915/gem/i915_gem_mman.h
index 25a3c4d6cd65..4d3b493e853a 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_mman.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_mman.h
@@ -24,5 +24,6 @@ void i915_mmap_offset_destroy(struct i915_mmap_offset *mmo, struct mutex *mutex)
void __i915_gem_object_release_mmap_gtt(struct drm_i915_gem_object *obj);
void i915_gem_object_release_mmap(struct drm_i915_gem_object *obj);
void i915_gem_object_release_mmap_offset(struct drm_i915_gem_object *obj);
+int i915_gem_mmap_gtt_version(void);
#endif
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object_types.h b/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
index 632d6b804cfb..07237fbfdaac 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
@@ -65,6 +65,9 @@ struct drm_i915_gem_object_ops {
enum i915_mmap_type {
I915_MMAP_TYPE_GTT = 0,
+ I915_MMAP_TYPE_WC,
+ I915_MMAP_TYPE_WB,
+ I915_MMAP_TYPE_UC,
};
struct i915_mmap_offset {
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 5fe071640893..ac6d4470ce75 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -61,6 +61,7 @@
#include "gem/i915_gem_context.h"
#include "gem/i915_gem_ioctls.h"
+#include "gem/i915_gem_mman.h"
#include "gt/intel_gt.h"
#include "gt/intel_gt_pm.h"
#include "gt/intel_rc6.h"
@@ -2714,7 +2715,7 @@ static const struct drm_ioctl_desc i915_ioctls[] = {
DRM_IOCTL_DEF_DRV(I915_GEM_PREAD, i915_gem_pread_ioctl, DRM_RENDER_ALLOW),
DRM_IOCTL_DEF_DRV(I915_GEM_PWRITE, i915_gem_pwrite_ioctl, DRM_RENDER_ALLOW),
DRM_IOCTL_DEF_DRV(I915_GEM_MMAP, i915_gem_mmap_ioctl, DRM_RENDER_ALLOW),
- DRM_IOCTL_DEF_DRV(I915_GEM_MMAP_GTT, i915_gem_mmap_gtt_ioctl, DRM_RENDER_ALLOW),
+ DRM_IOCTL_DEF_DRV(I915_GEM_MMAP_OFFSET, i915_gem_mmap_offset_ioctl, DRM_RENDER_ALLOW),
DRM_IOCTL_DEF_DRV(I915_GEM_SET_DOMAIN, i915_gem_set_domain_ioctl, DRM_RENDER_ALLOW),
DRM_IOCTL_DEF_DRV(I915_GEM_SW_FINISH, i915_gem_sw_finish_ioctl, DRM_RENDER_ALLOW),
DRM_IOCTL_DEF_DRV(I915_GEM_SET_TILING, i915_gem_set_tiling_ioctl, DRM_RENDER_ALLOW),
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index a37c77ba5c40..6ee4b86ca758 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1861,7 +1861,6 @@ i915_mutex_lock_interruptible(struct drm_device *dev)
int i915_gem_dumb_create(struct drm_file *file_priv,
struct drm_device *dev,
struct drm_mode_create_dumb *args);
-int i915_gem_mmap_gtt_version(void);
int __must_check i915_gem_set_global_seqno(struct drm_device *dev, u32 seqno);
diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index 5400d7e057f1..e844c3a8d197 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -395,6 +395,7 @@ typedef struct _drm_i915_sarea {
#define DRM_IOCTL_I915_GEM_PWRITE DRM_IOW (DRM_COMMAND_BASE + DRM_I915_GEM_PWRITE, struct drm_i915_gem_pwrite)
#define DRM_IOCTL_I915_GEM_MMAP DRM_IOWR(DRM_COMMAND_BASE + DRM_I915_GEM_MMAP, struct drm_i915_gem_mmap)
#define DRM_IOCTL_I915_GEM_MMAP_GTT DRM_IOWR(DRM_COMMAND_BASE + DRM_I915_GEM_MMAP_GTT, struct drm_i915_gem_mmap_gtt)
+#define DRM_IOCTL_I915_GEM_MMAP_OFFSET DRM_IOWR(DRM_COMMAND_BASE + DRM_I915_GEM_MMAP_GTT, struct drm_i915_gem_mmap_offset)
#define DRM_IOCTL_I915_GEM_SET_DOMAIN DRM_IOW (DRM_COMMAND_BASE + DRM_I915_GEM_SET_DOMAIN, struct drm_i915_gem_set_domain)
#define DRM_IOCTL_I915_GEM_SW_FINISH DRM_IOW (DRM_COMMAND_BASE + DRM_I915_GEM_SW_FINISH, struct drm_i915_gem_sw_finish)
#define DRM_IOCTL_I915_GEM_SET_TILING DRM_IOWR (DRM_COMMAND_BASE + DRM_I915_GEM_SET_TILING, struct drm_i915_gem_set_tiling)
@@ -793,6 +794,32 @@ struct drm_i915_gem_mmap_gtt {
__u64 offset;
};
+struct drm_i915_gem_mmap_offset {
+ /** Handle for the object being mapped. */
+ __u32 handle;
+ __u32 pad;
+ /**
+ * Fake offset to use for subsequent mmap call
+ *
+ * This is a fixed-size type for 32/64 compatibility.
+ */
+ __u64 offset;
+
+ /**
+ * Flags for extended behaviour.
+ *
+ * It is mandatory that either one of the MMAP_OFFSET flags
+ * should be passed here.
+ */
+ __u64 flags;
+#define I915_MMAP_OFFSET_GTT 0
+#define I915_MMAP_OFFSET_WC 1
+#define I915_MMAP_OFFSET_WB 2
+#define I915_MMAP_OFFSET_UC 3
+
+ __u64 extensions;
+};
+
struct drm_i915_gem_set_domain {
/** Handle for the object */
__u32 handle;
--
2.23.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 3/4] drm/i915: cpu-map based dumb buffers
@ 2019-11-15 11:45 ` Abdiel Janulgue
0 siblings, 0 replies; 28+ messages in thread
From: Abdiel Janulgue @ 2019-11-15 11:45 UTC (permalink / raw)
To: intel-gfx; +Cc: Matthew Auld
Prefer CPU WC mmaps via our new mmap offset plumbing otherwise fall-
back to GTT mmaps when hw doesn't support PAT
Signed-off-by: Abdiel Janulgue <abdiel.janulgue@linux.intel.com>
Cc: Matthew Auld <matthew.auld@intel.com>
---
drivers/gpu/drm/i915/gem/i915_gem_mman.c | 18 ++++++++++++++++++
drivers/gpu/drm/i915/gem/i915_gem_mman.h | 2 ++
drivers/gpu/drm/i915/i915_drv.c | 1 +
3 files changed, 21 insertions(+)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_mman.c b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
index d2ed8a463672..c1756e4f46b9 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_mman.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
@@ -540,6 +540,24 @@ __assign_gem_object_mmap_data(struct drm_file *file,
return ret;
}
+int
+i915_gem_mmap_dumb(struct drm_file *file,
+ struct drm_device *dev,
+ u32 handle,
+ u64 *offset)
+{
+ enum i915_mmap_type mmap_type;
+
+ if (boot_cpu_has(X86_FEATURE_PAT))
+ mmap_type = I915_MMAP_TYPE_WC;
+ else if (!i915_ggtt_has_aperture(&to_i915(dev)->ggtt))
+ return -ENODEV;
+ else
+ mmap_type = I915_MMAP_TYPE_GTT;
+
+ return __assign_gem_object_mmap_data(file, handle, mmap_type, offset);
+}
+
/**
* i915_gem_mmap_offset_ioctl - prepare an object for GTT mmap'ing
* @dev: DRM device
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_mman.h b/drivers/gpu/drm/i915/gem/i915_gem_mman.h
index 4d3b493e853a..6e70b91dabc4 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_mman.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_mman.h
@@ -24,6 +24,8 @@ void i915_mmap_offset_destroy(struct i915_mmap_offset *mmo, struct mutex *mutex)
void __i915_gem_object_release_mmap_gtt(struct drm_i915_gem_object *obj);
void i915_gem_object_release_mmap(struct drm_i915_gem_object *obj);
void i915_gem_object_release_mmap_offset(struct drm_i915_gem_object *obj);
+int i915_gem_mmap_dumb(struct drm_file *file_priv, struct drm_device *dev,
+ u32 handle, u64 *offset);
int i915_gem_mmap_gtt_version(void);
#endif
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index ac6d4470ce75..f7db0bbbe302 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -2767,6 +2767,7 @@ static struct drm_driver driver = {
.get_scanout_position = i915_get_crtc_scanoutpos,
.dumb_create = i915_gem_dumb_create,
+ .dumb_map_offset = i915_gem_mmap_dumb,
.ioctls = i915_ioctls,
.num_ioctls = ARRAY_SIZE(i915_ioctls),
.fops = &i915_driver_fops,
--
2.23.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [Intel-gfx] [PATCH 3/4] drm/i915: cpu-map based dumb buffers
@ 2019-11-15 11:45 ` Abdiel Janulgue
0 siblings, 0 replies; 28+ messages in thread
From: Abdiel Janulgue @ 2019-11-15 11:45 UTC (permalink / raw)
To: intel-gfx; +Cc: Matthew Auld
Prefer CPU WC mmaps via our new mmap offset plumbing otherwise fall-
back to GTT mmaps when hw doesn't support PAT
Signed-off-by: Abdiel Janulgue <abdiel.janulgue@linux.intel.com>
Cc: Matthew Auld <matthew.auld@intel.com>
---
drivers/gpu/drm/i915/gem/i915_gem_mman.c | 18 ++++++++++++++++++
drivers/gpu/drm/i915/gem/i915_gem_mman.h | 2 ++
drivers/gpu/drm/i915/i915_drv.c | 1 +
3 files changed, 21 insertions(+)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_mman.c b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
index d2ed8a463672..c1756e4f46b9 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_mman.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
@@ -540,6 +540,24 @@ __assign_gem_object_mmap_data(struct drm_file *file,
return ret;
}
+int
+i915_gem_mmap_dumb(struct drm_file *file,
+ struct drm_device *dev,
+ u32 handle,
+ u64 *offset)
+{
+ enum i915_mmap_type mmap_type;
+
+ if (boot_cpu_has(X86_FEATURE_PAT))
+ mmap_type = I915_MMAP_TYPE_WC;
+ else if (!i915_ggtt_has_aperture(&to_i915(dev)->ggtt))
+ return -ENODEV;
+ else
+ mmap_type = I915_MMAP_TYPE_GTT;
+
+ return __assign_gem_object_mmap_data(file, handle, mmap_type, offset);
+}
+
/**
* i915_gem_mmap_offset_ioctl - prepare an object for GTT mmap'ing
* @dev: DRM device
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_mman.h b/drivers/gpu/drm/i915/gem/i915_gem_mman.h
index 4d3b493e853a..6e70b91dabc4 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_mman.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_mman.h
@@ -24,6 +24,8 @@ void i915_mmap_offset_destroy(struct i915_mmap_offset *mmo, struct mutex *mutex)
void __i915_gem_object_release_mmap_gtt(struct drm_i915_gem_object *obj);
void i915_gem_object_release_mmap(struct drm_i915_gem_object *obj);
void i915_gem_object_release_mmap_offset(struct drm_i915_gem_object *obj);
+int i915_gem_mmap_dumb(struct drm_file *file_priv, struct drm_device *dev,
+ u32 handle, u64 *offset);
int i915_gem_mmap_gtt_version(void);
#endif
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index ac6d4470ce75..f7db0bbbe302 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -2767,6 +2767,7 @@ static struct drm_driver driver = {
.get_scanout_position = i915_get_crtc_scanoutpos,
.dumb_create = i915_gem_dumb_create,
+ .dumb_map_offset = i915_gem_mmap_dumb,
.ioctls = i915_ioctls,
.num_ioctls = ARRAY_SIZE(i915_ioctls),
.fops = &i915_driver_fops,
--
2.23.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 4/4] drm/i915: Add cpu fault handler for mmap_offset
@ 2019-11-15 11:45 ` Abdiel Janulgue
0 siblings, 0 replies; 28+ messages in thread
From: Abdiel Janulgue @ 2019-11-15 11:45 UTC (permalink / raw)
To: intel-gfx; +Cc: Matthew Auld
Fault handler to handle missing pages for shmem-backed objects.
v2: bail out of inserting PTEs when failing to insert the
fault address
v3: has struct page check
v4: Add self-test for validating CPU fault handler to ensure PTEs
are revoked when an object is unbound.
Signed-off-by: Abdiel Janulgue <abdiel.janulgue@linux.intel.com>
Signed-off-by: Matthew Auld <matthew.auld@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
drivers/gpu/drm/i915/gem/i915_gem_mman.c | 128 ++++++++++++++----
.../drm/i915/gem/selftests/i915_gem_mman.c | 47 +++++--
2 files changed, 141 insertions(+), 34 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_mman.c b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
index c1756e4f46b9..2f7ed2e6dfa8 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_mman.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
@@ -5,6 +5,7 @@
*/
#include <linux/mman.h>
+#include <linux/pfn_t.h>
#include <linux/sizes.h>
#include "gt/intel_gt.h"
@@ -201,6 +202,70 @@ compute_partial_view(const struct drm_i915_gem_object *obj,
return view;
}
+static vm_fault_t i915_error_to_vmf_fault(int err)
+{
+ switch (err) {
+ default:
+ WARN_ONCE(err, "unhandled error in %s: %i\n", __func__, err);
+ /* fallthrough */
+ case -EIO: /* shmemfs failure from swap device */
+ case -EFAULT: /* purged object */
+ case -ENODEV: /* bad object, how did you get here! */
+ return VM_FAULT_SIGBUS;
+
+ case -ENOSPC: /* shmemfs allocation failure */
+ case -ENOMEM: /* our allocation failure */
+ return VM_FAULT_OOM;
+
+ case 0:
+ case -EAGAIN:
+ case -ERESTARTSYS:
+ case -EINTR:
+ case -EBUSY:
+ /*
+ * EBUSY is ok: this just means that another thread
+ * already did the job.
+ */
+ return VM_FAULT_NOPAGE;
+ }
+}
+
+static vm_fault_t i915_gem_fault_cpu(struct vm_fault *vmf)
+{
+ struct vm_area_struct *area = vmf->vma;
+ struct i915_mmap_offset *priv = area->vm_private_data;
+ struct drm_i915_gem_object *obj = priv->obj;
+ vm_fault_t vmf_ret;
+ unsigned long i, size = area->vm_end - area->vm_start;
+ bool write = area->vm_flags & VM_WRITE;
+ int ret;
+
+ if (!i915_gem_object_has_struct_page(obj))
+ return VM_FAULT_SIGBUS;
+
+ /* Sanity check that we allow writing into this object */
+ if (i915_gem_object_is_readonly(obj) && write)
+ return VM_FAULT_SIGBUS;
+
+ ret = i915_gem_object_pin_pages(obj);
+ if (ret)
+ return i915_error_to_vmf_fault(ret);
+
+ for (i = 0; i < size >> PAGE_SHIFT; i++) {
+ struct page *page = i915_gem_object_get_page(obj, i);
+
+ vmf_ret = vmf_insert_pfn(area,
+ (unsigned long)area->vm_start + i * PAGE_SIZE,
+ page_to_pfn(page));
+ if (vmf_ret != VM_FAULT_NOPAGE)
+ break;
+ }
+
+ i915_gem_object_unpin_pages(obj);
+
+ return vmf_ret;
+}
+
/**
* i915_gem_fault - fault a page into the GTT
* @vmf: fault info
@@ -340,30 +405,7 @@ vm_fault_t i915_gem_fault(struct vm_fault *vmf)
intel_runtime_pm_put(rpm, wakeref);
i915_gem_object_unpin_pages(obj);
err:
- switch (ret) {
- default:
- WARN_ONCE(ret, "unhandled error in %s: %i\n", __func__, ret);
- /* fallthrough */
- case -EIO: /* shmemfs failure from swap device */
- case -EFAULT: /* purged object */
- case -ENODEV: /* bad object, how did you get here! */
- return VM_FAULT_SIGBUS;
-
- case -ENOSPC: /* shmemfs allocation failure */
- case -ENOMEM: /* our allocation failure */
- return VM_FAULT_OOM;
-
- case 0:
- case -EAGAIN:
- case -ERESTARTSYS:
- case -EINTR:
- case -EBUSY:
- /*
- * EBUSY is ok: this just means that another thread
- * already did the job.
- */
- return VM_FAULT_NOPAGE;
- }
+ return i915_error_to_vmf_fault(ret);
}
void __i915_gem_object_release_mmap_gtt(struct drm_i915_gem_object *obj)
@@ -650,6 +692,33 @@ static const struct vm_operations_struct i915_gem_gtt_vm_ops = {
.close = i915_gem_vm_close,
};
+static const struct vm_operations_struct i915_gem_cpu_vm_ops = {
+ .fault = i915_gem_fault_cpu,
+ .open = i915_gem_vm_open,
+ .close = i915_gem_vm_close,
+};
+
+static void set_vmdata_mmap_offset(struct i915_mmap_offset *mmo, struct vm_area_struct *vma)
+{
+ switch (mmo->mmap_type) {
+ case I915_MMAP_TYPE_WC:
+ vma->vm_page_prot =
+ pgprot_writecombine(vm_get_page_prot(vma->vm_flags));
+ break;
+ case I915_MMAP_TYPE_WB:
+ vma->vm_page_prot = vm_get_page_prot(vma->vm_flags);
+ break;
+ case I915_MMAP_TYPE_UC:
+ vma->vm_page_prot =
+ pgprot_noncached(vm_get_page_prot(vma->vm_flags));
+ break;
+ default:
+ break;
+ }
+
+ vma->vm_ops = &i915_gem_cpu_vm_ops;
+}
+
/* This overcomes the limitation in drm_gem_mmap's assignment of a
* drm_gem_object as the vma->vm_private_data. Since we need to
* be able to resolve multiple mmap offsets which could be tied
@@ -717,7 +786,16 @@ int i915_gem_mmap(struct file *filp, struct vm_area_struct *vma)
vma->vm_page_prot = pgprot_decrypted(vma->vm_page_prot);
vma->vm_private_data = mmo;
- vma->vm_ops = &i915_gem_gtt_vm_ops;
+ switch (mmo->mmap_type) {
+ case I915_MMAP_TYPE_WC:
+ case I915_MMAP_TYPE_WB:
+ case I915_MMAP_TYPE_UC:
+ set_vmdata_mmap_offset(mmo, vma);
+ break;
+ case I915_MMAP_TYPE_GTT:
+ vma->vm_ops = &i915_gem_gtt_vm_ops;
+ break;
+ }
return 0;
}
diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c b/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c
index 63e24f6b0d40..1bedc9d115c5 100644
--- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c
+++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c
@@ -731,11 +731,13 @@ static int igt_mmap_offset_exhaustion(void *arg)
}
#define expand32(x) (((x) << 0) | ((x) << 8) | ((x) << 16) | ((x) << 24))
-static int igt_mmap_gtt(void *arg)
+static int igt_mmap(void *arg, enum i915_mmap_type type)
{
struct drm_i915_private *i915 = arg;
struct drm_i915_gem_object *obj;
struct vm_area_struct *area;
+ /* Ownership transferred to parent gem object in create_mmap_offset */
+ struct i915_mmap_offset *mmo = kzalloc(sizeof(*mmo), GFP_KERNEL);
unsigned long addr;
void *vaddr;
int err, i;
@@ -756,11 +758,12 @@ static int igt_mmap_gtt(void *arg)
i915_gem_object_flush_map(obj);
i915_gem_object_unpin_map(obj);
- err = create_mmap_offset(obj);
+ err = create_mmap_offset(obj, mmo);
if (err)
goto out;
- addr = igt_mmap_node(i915, &obj->base.vma_node,
+ mmo->mmap_type = type;
+ addr = igt_mmap_node(i915, &mmo->vma_node,
0, PROT_WRITE, MAP_SHARED);
if (IS_ERR_VALUE(addr)) {
err = addr;
@@ -776,8 +779,8 @@ static int igt_mmap_gtt(void *arg)
goto out_unmap;
}
- if (area->vm_private_data != obj) {
- pr_err("vm_area_struct did not point back to our object!\n");
+ if (area->vm_private_data != mmo) {
+ pr_err("vm_area_struct did not point back to our mmap_offset object!\n");
err = -EINVAL;
goto out_unmap;
}
@@ -828,6 +831,16 @@ static int igt_mmap_gtt(void *arg)
return err;
}
+static int igt_mmap_gtt(void *arg)
+{
+ return igt_mmap(arg, I915_MMAP_TYPE_GTT);
+}
+
+static int igt_mmap_cpu(void *arg)
+{
+ return igt_mmap(arg, I915_MMAP_TYPE_WC);
+}
+
static int check_present_pte(pte_t *pte, unsigned long addr, void *data)
{
if (!pte_present(*pte) || pte_none(*pte)) {
@@ -880,10 +893,12 @@ static int prefault_range(u64 start, u64 len)
return __get_user(c, end - 1);
}
-static int igt_mmap_gtt_revoke(void *arg)
+static int igt_mmap_revoke(void *arg, enum i915_mmap_type type)
{
struct drm_i915_private *i915 = arg;
struct drm_i915_gem_object *obj;
+ /* Ownership transferred to parent gem object in create_mmap_offset */
+ struct i915_mmap_offset *mmo = kzalloc(sizeof(*mmo), GFP_KERNEL);
unsigned long addr;
int err;
@@ -894,11 +909,12 @@ static int igt_mmap_gtt_revoke(void *arg)
if (IS_ERR(obj))
return PTR_ERR(obj);
- err = create_mmap_offset(obj);
+ err = create_mmap_offset(obj, mmo);
if (err)
goto out;
- addr = igt_mmap_node(i915, &obj->base.vma_node,
+ mmo->mmap_type = type;
+ addr = igt_mmap_node(i915, &mmo->vma_node,
0, PROT_WRITE, MAP_SHARED);
if (IS_ERR_VALUE(addr)) {
err = addr;
@@ -909,7 +925,8 @@ static int igt_mmap_gtt_revoke(void *arg)
if (err)
goto out_unmap;
- GEM_BUG_ON(!atomic_read(&obj->bind_count));
+ GEM_BUG_ON(mmo->mmap_type == I915_MMAP_TYPE_GTT &&
+ !atomic_read(&obj->bind_count));
err = check_present(addr, obj->base.size);
if (err)
@@ -947,6 +964,16 @@ static int igt_mmap_gtt_revoke(void *arg)
return err;
}
+static int igt_mmap_gtt_revoke(void *arg)
+{
+ return igt_mmap_revoke(arg, I915_MMAP_TYPE_GTT);
+}
+
+static int igt_mmap_cpu_revoke(void *arg)
+{
+ return igt_mmap_revoke(arg, I915_MMAP_TYPE_WC);
+}
+
int i915_gem_mman_live_selftests(struct drm_i915_private *i915)
{
static const struct i915_subtest tests[] = {
@@ -954,7 +981,9 @@ int i915_gem_mman_live_selftests(struct drm_i915_private *i915)
SUBTEST(igt_smoke_tiling),
SUBTEST(igt_mmap_offset_exhaustion),
SUBTEST(igt_mmap_gtt),
+ SUBTEST(igt_mmap_cpu),
SUBTEST(igt_mmap_gtt_revoke),
+ SUBTEST(igt_mmap_cpu_revoke),
};
return i915_subtests(tests, i915);
--
2.23.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [Intel-gfx] [PATCH 4/4] drm/i915: Add cpu fault handler for mmap_offset
@ 2019-11-15 11:45 ` Abdiel Janulgue
0 siblings, 0 replies; 28+ messages in thread
From: Abdiel Janulgue @ 2019-11-15 11:45 UTC (permalink / raw)
To: intel-gfx; +Cc: Matthew Auld
Fault handler to handle missing pages for shmem-backed objects.
v2: bail out of inserting PTEs when failing to insert the
fault address
v3: has struct page check
v4: Add self-test for validating CPU fault handler to ensure PTEs
are revoked when an object is unbound.
Signed-off-by: Abdiel Janulgue <abdiel.janulgue@linux.intel.com>
Signed-off-by: Matthew Auld <matthew.auld@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
drivers/gpu/drm/i915/gem/i915_gem_mman.c | 128 ++++++++++++++----
.../drm/i915/gem/selftests/i915_gem_mman.c | 47 +++++--
2 files changed, 141 insertions(+), 34 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_mman.c b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
index c1756e4f46b9..2f7ed2e6dfa8 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_mman.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
@@ -5,6 +5,7 @@
*/
#include <linux/mman.h>
+#include <linux/pfn_t.h>
#include <linux/sizes.h>
#include "gt/intel_gt.h"
@@ -201,6 +202,70 @@ compute_partial_view(const struct drm_i915_gem_object *obj,
return view;
}
+static vm_fault_t i915_error_to_vmf_fault(int err)
+{
+ switch (err) {
+ default:
+ WARN_ONCE(err, "unhandled error in %s: %i\n", __func__, err);
+ /* fallthrough */
+ case -EIO: /* shmemfs failure from swap device */
+ case -EFAULT: /* purged object */
+ case -ENODEV: /* bad object, how did you get here! */
+ return VM_FAULT_SIGBUS;
+
+ case -ENOSPC: /* shmemfs allocation failure */
+ case -ENOMEM: /* our allocation failure */
+ return VM_FAULT_OOM;
+
+ case 0:
+ case -EAGAIN:
+ case -ERESTARTSYS:
+ case -EINTR:
+ case -EBUSY:
+ /*
+ * EBUSY is ok: this just means that another thread
+ * already did the job.
+ */
+ return VM_FAULT_NOPAGE;
+ }
+}
+
+static vm_fault_t i915_gem_fault_cpu(struct vm_fault *vmf)
+{
+ struct vm_area_struct *area = vmf->vma;
+ struct i915_mmap_offset *priv = area->vm_private_data;
+ struct drm_i915_gem_object *obj = priv->obj;
+ vm_fault_t vmf_ret;
+ unsigned long i, size = area->vm_end - area->vm_start;
+ bool write = area->vm_flags & VM_WRITE;
+ int ret;
+
+ if (!i915_gem_object_has_struct_page(obj))
+ return VM_FAULT_SIGBUS;
+
+ /* Sanity check that we allow writing into this object */
+ if (i915_gem_object_is_readonly(obj) && write)
+ return VM_FAULT_SIGBUS;
+
+ ret = i915_gem_object_pin_pages(obj);
+ if (ret)
+ return i915_error_to_vmf_fault(ret);
+
+ for (i = 0; i < size >> PAGE_SHIFT; i++) {
+ struct page *page = i915_gem_object_get_page(obj, i);
+
+ vmf_ret = vmf_insert_pfn(area,
+ (unsigned long)area->vm_start + i * PAGE_SIZE,
+ page_to_pfn(page));
+ if (vmf_ret != VM_FAULT_NOPAGE)
+ break;
+ }
+
+ i915_gem_object_unpin_pages(obj);
+
+ return vmf_ret;
+}
+
/**
* i915_gem_fault - fault a page into the GTT
* @vmf: fault info
@@ -340,30 +405,7 @@ vm_fault_t i915_gem_fault(struct vm_fault *vmf)
intel_runtime_pm_put(rpm, wakeref);
i915_gem_object_unpin_pages(obj);
err:
- switch (ret) {
- default:
- WARN_ONCE(ret, "unhandled error in %s: %i\n", __func__, ret);
- /* fallthrough */
- case -EIO: /* shmemfs failure from swap device */
- case -EFAULT: /* purged object */
- case -ENODEV: /* bad object, how did you get here! */
- return VM_FAULT_SIGBUS;
-
- case -ENOSPC: /* shmemfs allocation failure */
- case -ENOMEM: /* our allocation failure */
- return VM_FAULT_OOM;
-
- case 0:
- case -EAGAIN:
- case -ERESTARTSYS:
- case -EINTR:
- case -EBUSY:
- /*
- * EBUSY is ok: this just means that another thread
- * already did the job.
- */
- return VM_FAULT_NOPAGE;
- }
+ return i915_error_to_vmf_fault(ret);
}
void __i915_gem_object_release_mmap_gtt(struct drm_i915_gem_object *obj)
@@ -650,6 +692,33 @@ static const struct vm_operations_struct i915_gem_gtt_vm_ops = {
.close = i915_gem_vm_close,
};
+static const struct vm_operations_struct i915_gem_cpu_vm_ops = {
+ .fault = i915_gem_fault_cpu,
+ .open = i915_gem_vm_open,
+ .close = i915_gem_vm_close,
+};
+
+static void set_vmdata_mmap_offset(struct i915_mmap_offset *mmo, struct vm_area_struct *vma)
+{
+ switch (mmo->mmap_type) {
+ case I915_MMAP_TYPE_WC:
+ vma->vm_page_prot =
+ pgprot_writecombine(vm_get_page_prot(vma->vm_flags));
+ break;
+ case I915_MMAP_TYPE_WB:
+ vma->vm_page_prot = vm_get_page_prot(vma->vm_flags);
+ break;
+ case I915_MMAP_TYPE_UC:
+ vma->vm_page_prot =
+ pgprot_noncached(vm_get_page_prot(vma->vm_flags));
+ break;
+ default:
+ break;
+ }
+
+ vma->vm_ops = &i915_gem_cpu_vm_ops;
+}
+
/* This overcomes the limitation in drm_gem_mmap's assignment of a
* drm_gem_object as the vma->vm_private_data. Since we need to
* be able to resolve multiple mmap offsets which could be tied
@@ -717,7 +786,16 @@ int i915_gem_mmap(struct file *filp, struct vm_area_struct *vma)
vma->vm_page_prot = pgprot_decrypted(vma->vm_page_prot);
vma->vm_private_data = mmo;
- vma->vm_ops = &i915_gem_gtt_vm_ops;
+ switch (mmo->mmap_type) {
+ case I915_MMAP_TYPE_WC:
+ case I915_MMAP_TYPE_WB:
+ case I915_MMAP_TYPE_UC:
+ set_vmdata_mmap_offset(mmo, vma);
+ break;
+ case I915_MMAP_TYPE_GTT:
+ vma->vm_ops = &i915_gem_gtt_vm_ops;
+ break;
+ }
return 0;
}
diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c b/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c
index 63e24f6b0d40..1bedc9d115c5 100644
--- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c
+++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c
@@ -731,11 +731,13 @@ static int igt_mmap_offset_exhaustion(void *arg)
}
#define expand32(x) (((x) << 0) | ((x) << 8) | ((x) << 16) | ((x) << 24))
-static int igt_mmap_gtt(void *arg)
+static int igt_mmap(void *arg, enum i915_mmap_type type)
{
struct drm_i915_private *i915 = arg;
struct drm_i915_gem_object *obj;
struct vm_area_struct *area;
+ /* Ownership transferred to parent gem object in create_mmap_offset */
+ struct i915_mmap_offset *mmo = kzalloc(sizeof(*mmo), GFP_KERNEL);
unsigned long addr;
void *vaddr;
int err, i;
@@ -756,11 +758,12 @@ static int igt_mmap_gtt(void *arg)
i915_gem_object_flush_map(obj);
i915_gem_object_unpin_map(obj);
- err = create_mmap_offset(obj);
+ err = create_mmap_offset(obj, mmo);
if (err)
goto out;
- addr = igt_mmap_node(i915, &obj->base.vma_node,
+ mmo->mmap_type = type;
+ addr = igt_mmap_node(i915, &mmo->vma_node,
0, PROT_WRITE, MAP_SHARED);
if (IS_ERR_VALUE(addr)) {
err = addr;
@@ -776,8 +779,8 @@ static int igt_mmap_gtt(void *arg)
goto out_unmap;
}
- if (area->vm_private_data != obj) {
- pr_err("vm_area_struct did not point back to our object!\n");
+ if (area->vm_private_data != mmo) {
+ pr_err("vm_area_struct did not point back to our mmap_offset object!\n");
err = -EINVAL;
goto out_unmap;
}
@@ -828,6 +831,16 @@ static int igt_mmap_gtt(void *arg)
return err;
}
+static int igt_mmap_gtt(void *arg)
+{
+ return igt_mmap(arg, I915_MMAP_TYPE_GTT);
+}
+
+static int igt_mmap_cpu(void *arg)
+{
+ return igt_mmap(arg, I915_MMAP_TYPE_WC);
+}
+
static int check_present_pte(pte_t *pte, unsigned long addr, void *data)
{
if (!pte_present(*pte) || pte_none(*pte)) {
@@ -880,10 +893,12 @@ static int prefault_range(u64 start, u64 len)
return __get_user(c, end - 1);
}
-static int igt_mmap_gtt_revoke(void *arg)
+static int igt_mmap_revoke(void *arg, enum i915_mmap_type type)
{
struct drm_i915_private *i915 = arg;
struct drm_i915_gem_object *obj;
+ /* Ownership transferred to parent gem object in create_mmap_offset */
+ struct i915_mmap_offset *mmo = kzalloc(sizeof(*mmo), GFP_KERNEL);
unsigned long addr;
int err;
@@ -894,11 +909,12 @@ static int igt_mmap_gtt_revoke(void *arg)
if (IS_ERR(obj))
return PTR_ERR(obj);
- err = create_mmap_offset(obj);
+ err = create_mmap_offset(obj, mmo);
if (err)
goto out;
- addr = igt_mmap_node(i915, &obj->base.vma_node,
+ mmo->mmap_type = type;
+ addr = igt_mmap_node(i915, &mmo->vma_node,
0, PROT_WRITE, MAP_SHARED);
if (IS_ERR_VALUE(addr)) {
err = addr;
@@ -909,7 +925,8 @@ static int igt_mmap_gtt_revoke(void *arg)
if (err)
goto out_unmap;
- GEM_BUG_ON(!atomic_read(&obj->bind_count));
+ GEM_BUG_ON(mmo->mmap_type == I915_MMAP_TYPE_GTT &&
+ !atomic_read(&obj->bind_count));
err = check_present(addr, obj->base.size);
if (err)
@@ -947,6 +964,16 @@ static int igt_mmap_gtt_revoke(void *arg)
return err;
}
+static int igt_mmap_gtt_revoke(void *arg)
+{
+ return igt_mmap_revoke(arg, I915_MMAP_TYPE_GTT);
+}
+
+static int igt_mmap_cpu_revoke(void *arg)
+{
+ return igt_mmap_revoke(arg, I915_MMAP_TYPE_WC);
+}
+
int i915_gem_mman_live_selftests(struct drm_i915_private *i915)
{
static const struct i915_subtest tests[] = {
@@ -954,7 +981,9 @@ int i915_gem_mman_live_selftests(struct drm_i915_private *i915)
SUBTEST(igt_smoke_tiling),
SUBTEST(igt_mmap_offset_exhaustion),
SUBTEST(igt_mmap_gtt),
+ SUBTEST(igt_mmap_cpu),
SUBTEST(igt_mmap_gtt_revoke),
+ SUBTEST(igt_mmap_cpu_revoke),
};
return i915_subtests(tests, i915);
--
2.23.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 28+ messages in thread
* Re: [PATCH 2/4] drm/i915: Introduce DRM_I915_GEM_MMAP_OFFSET
@ 2019-11-15 13:51 ` Chris Wilson
0 siblings, 0 replies; 28+ messages in thread
From: Chris Wilson @ 2019-11-15 13:51 UTC (permalink / raw)
To: Abdiel Janulgue, intel-gfx; +Cc: Matthew Auld
Quoting Abdiel Janulgue (2019-11-15 11:45:47)
> +i915_gem_mmap_offset_ioctl(struct drm_device *dev, void *data,
> + struct drm_file *file)
> {
> - struct drm_i915_gem_mmap_gtt *args = data;
> + struct drm_i915_private *i915 = to_i915(dev);
> + struct drm_i915_gem_mmap_offset *args = data;
> + enum i915_mmap_type type;
> +
> + switch (args->flags) {
> + case I915_MMAP_OFFSET_GTT:
> + if (!i915_ggtt_has_aperture(&i915->ggtt))
> + return -ENODEV;
> + type = I915_MMAP_TYPE_GTT;
> + break;
> +
> + case I915_MMAP_OFFSET_WC:
> + if (!boot_cpu_has(X86_FEATURE_PAT))
> + return -ENODEV;
> + type = I915_MMAP_TYPE_WC;
> + break;
> +
> + case I915_MMAP_OFFSET_WB:
> + type = I915_MMAP_TYPE_WB;
> + break;
> +
> + case I915_MMAP_OFFSET_UC:
> + if (!boot_cpu_has(X86_FEATURE_PAT))
> + return -ENODEV;
> + type = I915_MMAP_TYPE_UC;
> + break;
> +
> + default:
> + return -EINVAL;
> + }
>
> return __assign_gem_object_mmap_data(file, args->handle,
> I915_MMAP_TYPE_GTT,
s/TYPE_GTT/type/?
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [Intel-gfx] [PATCH 2/4] drm/i915: Introduce DRM_I915_GEM_MMAP_OFFSET
@ 2019-11-15 13:51 ` Chris Wilson
0 siblings, 0 replies; 28+ messages in thread
From: Chris Wilson @ 2019-11-15 13:51 UTC (permalink / raw)
To: Abdiel Janulgue, intel-gfx; +Cc: Matthew Auld
Quoting Abdiel Janulgue (2019-11-15 11:45:47)
> +i915_gem_mmap_offset_ioctl(struct drm_device *dev, void *data,
> + struct drm_file *file)
> {
> - struct drm_i915_gem_mmap_gtt *args = data;
> + struct drm_i915_private *i915 = to_i915(dev);
> + struct drm_i915_gem_mmap_offset *args = data;
> + enum i915_mmap_type type;
> +
> + switch (args->flags) {
> + case I915_MMAP_OFFSET_GTT:
> + if (!i915_ggtt_has_aperture(&i915->ggtt))
> + return -ENODEV;
> + type = I915_MMAP_TYPE_GTT;
> + break;
> +
> + case I915_MMAP_OFFSET_WC:
> + if (!boot_cpu_has(X86_FEATURE_PAT))
> + return -ENODEV;
> + type = I915_MMAP_TYPE_WC;
> + break;
> +
> + case I915_MMAP_OFFSET_WB:
> + type = I915_MMAP_TYPE_WB;
> + break;
> +
> + case I915_MMAP_OFFSET_UC:
> + if (!boot_cpu_has(X86_FEATURE_PAT))
> + return -ENODEV;
> + type = I915_MMAP_TYPE_UC;
> + break;
> +
> + default:
> + return -EINVAL;
> + }
>
> return __assign_gem_object_mmap_data(file, args->handle,
> I915_MMAP_TYPE_GTT,
s/TYPE_GTT/type/?
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 3/4] drm/i915: cpu-map based dumb buffers
@ 2019-11-15 13:54 ` Chris Wilson
0 siblings, 0 replies; 28+ messages in thread
From: Chris Wilson @ 2019-11-15 13:54 UTC (permalink / raw)
To: Abdiel Janulgue, intel-gfx; +Cc: Matthew Auld
Quoting Abdiel Janulgue (2019-11-15 11:45:48)
> Prefer CPU WC mmaps via our new mmap offset plumbing otherwise fall-
> back to GTT mmaps when hw doesn't support PAT
>
> Signed-off-by: Abdiel Janulgue <abdiel.janulgue@linux.intel.com>
> Cc: Matthew Auld <matthew.auld@intel.com>
> ---
> drivers/gpu/drm/i915/gem/i915_gem_mman.c | 18 ++++++++++++++++++
> drivers/gpu/drm/i915/gem/i915_gem_mman.h | 2 ++
> drivers/gpu/drm/i915/i915_drv.c | 1 +
> 3 files changed, 21 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_mman.c b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
> index d2ed8a463672..c1756e4f46b9 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_mman.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
> @@ -540,6 +540,24 @@ __assign_gem_object_mmap_data(struct drm_file *file,
> return ret;
> }
>
> +int
> +i915_gem_mmap_dumb(struct drm_file *file,
> + struct drm_device *dev,
> + u32 handle,
> + u64 *offset)
> +{
> + enum i915_mmap_type mmap_type;
> +
> + if (boot_cpu_has(X86_FEATURE_PAT))
> + mmap_type = I915_MMAP_TYPE_WC;
> + else if (!i915_ggtt_has_aperture(&to_i915(dev)->ggtt))
> + return -ENODEV;
> + else
> + mmap_type = I915_MMAP_TYPE_GTT;
> +
> + return __assign_gem_object_mmap_data(file, handle, mmap_type, offset);
Looks ok. Just a few nagging doubts about potential existing misuse by
userspace, such as are very using tiling on their dumb buffer, are they
passing in a non-dumb handle?
Of course we will need to beat igt into shape first, as a few pipe-crc
results suggests some not smart use of dumb buffers.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [Intel-gfx] [PATCH 3/4] drm/i915: cpu-map based dumb buffers
@ 2019-11-15 13:54 ` Chris Wilson
0 siblings, 0 replies; 28+ messages in thread
From: Chris Wilson @ 2019-11-15 13:54 UTC (permalink / raw)
To: Abdiel Janulgue, intel-gfx; +Cc: Matthew Auld
Quoting Abdiel Janulgue (2019-11-15 11:45:48)
> Prefer CPU WC mmaps via our new mmap offset plumbing otherwise fall-
> back to GTT mmaps when hw doesn't support PAT
>
> Signed-off-by: Abdiel Janulgue <abdiel.janulgue@linux.intel.com>
> Cc: Matthew Auld <matthew.auld@intel.com>
> ---
> drivers/gpu/drm/i915/gem/i915_gem_mman.c | 18 ++++++++++++++++++
> drivers/gpu/drm/i915/gem/i915_gem_mman.h | 2 ++
> drivers/gpu/drm/i915/i915_drv.c | 1 +
> 3 files changed, 21 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_mman.c b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
> index d2ed8a463672..c1756e4f46b9 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_mman.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
> @@ -540,6 +540,24 @@ __assign_gem_object_mmap_data(struct drm_file *file,
> return ret;
> }
>
> +int
> +i915_gem_mmap_dumb(struct drm_file *file,
> + struct drm_device *dev,
> + u32 handle,
> + u64 *offset)
> +{
> + enum i915_mmap_type mmap_type;
> +
> + if (boot_cpu_has(X86_FEATURE_PAT))
> + mmap_type = I915_MMAP_TYPE_WC;
> + else if (!i915_ggtt_has_aperture(&to_i915(dev)->ggtt))
> + return -ENODEV;
> + else
> + mmap_type = I915_MMAP_TYPE_GTT;
> +
> + return __assign_gem_object_mmap_data(file, handle, mmap_type, offset);
Looks ok. Just a few nagging doubts about potential existing misuse by
userspace, such as are very using tiling on their dumb buffer, are they
passing in a non-dumb handle?
Of course we will need to beat igt into shape first, as a few pipe-crc
results suggests some not smart use of dumb buffers.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 28+ messages in thread
* ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/4] drm/i915: Allow i915 to manage the vma offset nodes instead of drm core
@ 2019-11-15 13:56 ` Patchwork
0 siblings, 0 replies; 28+ messages in thread
From: Patchwork @ 2019-11-15 13:56 UTC (permalink / raw)
To: Abdiel Janulgue; +Cc: intel-gfx
== Series Details ==
Series: series starting with [1/4] drm/i915: Allow i915 to manage the vma offset nodes instead of drm core
URL : https://patchwork.freedesktop.org/series/69522/
State : warning
== Summary ==
$ dim checkpatch origin/drm-tip
a0ff7bbb56ea drm/i915: Allow i915 to manage the vma offset nodes instead of drm core
-:368: CHECK:BRACES: Blank lines aren't necessary before a close brace '}'
#368: FILE: drivers/gpu/drm/i915/gem/i915_gem_mman.c:646:
+
+ }
-:399: WARNING:FILE_PATH_CHANGES: added, moved or deleted file(s), does MAINTAINERS need updating?
#399:
new file mode 100644
-:404: WARNING:SPDX_LICENSE_TAG: Missing or malformed SPDX-License-Identifier tag in line 1
#404: FILE: drivers/gpu/drm/i915/gem/i915_gem_mman.h:1:
+/*
-:405: WARNING:SPDX_LICENSE_TAG: Misplaced SPDX-License-Identifier tag - use line 1 instead
#405: FILE: drivers/gpu/drm/i915/gem/i915_gem_mman.h:2:
+ * SPDX-License-Identifier: MIT
-:600: CHECK:LINE_SPACING: Please don't use multiple blank lines
#600: FILE: drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c:580:
+
-:747: WARNING:SPDX_LICENSE_TAG: Misplaced SPDX-License-Identifier tag - use line 1 instead
#747: FILE: drivers/gpu/drm/i915/i915_getparam.c:2:
* SPDX-License-Identifier: MIT
total: 0 errors, 4 warnings, 2 checks, 648 lines checked
bb6e6f21d2f9 drm/i915: Introduce DRM_I915_GEM_MMAP_OFFSET
-:187: WARNING:LONG_LINE: line over 100 characters
#187: FILE: include/uapi/drm/i915_drm.h:398:
+#define DRM_IOCTL_I915_GEM_MMAP_OFFSET DRM_IOWR(DRM_COMMAND_BASE + DRM_I915_GEM_MMAP_GTT, struct drm_i915_gem_mmap_offset)
total: 0 errors, 1 warnings, 0 checks, 153 lines checked
52a0eee7cfa2 drm/i915: cpu-map based dumb buffers
-:22: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#22: FILE: drivers/gpu/drm/i915/gem/i915_gem_mman.c:545:
+i915_gem_mmap_dumb(struct drm_file *file,
+ struct drm_device *dev,
-:32: WARNING:UNNECESSARY_ELSE: else is not generally useful after a break or return
#32: FILE: drivers/gpu/drm/i915/gem/i915_gem_mman.c:555:
+ return -ENODEV;
+ else
-:50: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#50: FILE: drivers/gpu/drm/i915/gem/i915_gem_mman.h:28:
+int i915_gem_mmap_dumb(struct drm_file *file_priv, struct drm_device *dev,
+ u32 handle, u64 *offset);
total: 0 errors, 1 warnings, 2 checks, 39 lines checked
27a373a960f1 drm/i915: Add cpu fault handler for mmap_offset
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 28+ messages in thread
* [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/4] drm/i915: Allow i915 to manage the vma offset nodes instead of drm core
@ 2019-11-15 13:56 ` Patchwork
0 siblings, 0 replies; 28+ messages in thread
From: Patchwork @ 2019-11-15 13:56 UTC (permalink / raw)
To: Abdiel Janulgue; +Cc: intel-gfx
== Series Details ==
Series: series starting with [1/4] drm/i915: Allow i915 to manage the vma offset nodes instead of drm core
URL : https://patchwork.freedesktop.org/series/69522/
State : warning
== Summary ==
$ dim checkpatch origin/drm-tip
a0ff7bbb56ea drm/i915: Allow i915 to manage the vma offset nodes instead of drm core
-:368: CHECK:BRACES: Blank lines aren't necessary before a close brace '}'
#368: FILE: drivers/gpu/drm/i915/gem/i915_gem_mman.c:646:
+
+ }
-:399: WARNING:FILE_PATH_CHANGES: added, moved or deleted file(s), does MAINTAINERS need updating?
#399:
new file mode 100644
-:404: WARNING:SPDX_LICENSE_TAG: Missing or malformed SPDX-License-Identifier tag in line 1
#404: FILE: drivers/gpu/drm/i915/gem/i915_gem_mman.h:1:
+/*
-:405: WARNING:SPDX_LICENSE_TAG: Misplaced SPDX-License-Identifier tag - use line 1 instead
#405: FILE: drivers/gpu/drm/i915/gem/i915_gem_mman.h:2:
+ * SPDX-License-Identifier: MIT
-:600: CHECK:LINE_SPACING: Please don't use multiple blank lines
#600: FILE: drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c:580:
+
-:747: WARNING:SPDX_LICENSE_TAG: Misplaced SPDX-License-Identifier tag - use line 1 instead
#747: FILE: drivers/gpu/drm/i915/i915_getparam.c:2:
* SPDX-License-Identifier: MIT
total: 0 errors, 4 warnings, 2 checks, 648 lines checked
bb6e6f21d2f9 drm/i915: Introduce DRM_I915_GEM_MMAP_OFFSET
-:187: WARNING:LONG_LINE: line over 100 characters
#187: FILE: include/uapi/drm/i915_drm.h:398:
+#define DRM_IOCTL_I915_GEM_MMAP_OFFSET DRM_IOWR(DRM_COMMAND_BASE + DRM_I915_GEM_MMAP_GTT, struct drm_i915_gem_mmap_offset)
total: 0 errors, 1 warnings, 0 checks, 153 lines checked
52a0eee7cfa2 drm/i915: cpu-map based dumb buffers
-:22: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#22: FILE: drivers/gpu/drm/i915/gem/i915_gem_mman.c:545:
+i915_gem_mmap_dumb(struct drm_file *file,
+ struct drm_device *dev,
-:32: WARNING:UNNECESSARY_ELSE: else is not generally useful after a break or return
#32: FILE: drivers/gpu/drm/i915/gem/i915_gem_mman.c:555:
+ return -ENODEV;
+ else
-:50: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#50: FILE: drivers/gpu/drm/i915/gem/i915_gem_mman.h:28:
+int i915_gem_mmap_dumb(struct drm_file *file_priv, struct drm_device *dev,
+ u32 handle, u64 *offset);
total: 0 errors, 1 warnings, 2 checks, 39 lines checked
27a373a960f1 drm/i915: Add cpu fault handler for mmap_offset
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 28+ messages in thread
* ✗ Fi.CI.SPARSE: warning for series starting with [1/4] drm/i915: Allow i915 to manage the vma offset nodes instead of drm core
@ 2019-11-15 13:57 ` Patchwork
0 siblings, 0 replies; 28+ messages in thread
From: Patchwork @ 2019-11-15 13:57 UTC (permalink / raw)
To: Abdiel Janulgue; +Cc: intel-gfx
== Series Details ==
Series: series starting with [1/4] drm/i915: Allow i915 to manage the vma offset nodes instead of drm core
URL : https://patchwork.freedesktop.org/series/69522/
State : warning
== Summary ==
$ dim sparse origin/drm-tip
Sparse version: v0.6.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 28+ messages in thread
* [Intel-gfx] ✗ Fi.CI.SPARSE: warning for series starting with [1/4] drm/i915: Allow i915 to manage the vma offset nodes instead of drm core
@ 2019-11-15 13:57 ` Patchwork
0 siblings, 0 replies; 28+ messages in thread
From: Patchwork @ 2019-11-15 13:57 UTC (permalink / raw)
To: Abdiel Janulgue; +Cc: intel-gfx
== Series Details ==
Series: series starting with [1/4] drm/i915: Allow i915 to manage the vma offset nodes instead of drm core
URL : https://patchwork.freedesktop.org/series/69522/
State : warning
== Summary ==
$ dim sparse origin/drm-tip
Sparse version: v0.6.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 4/4] drm/i915: Add cpu fault handler for mmap_offset
@ 2019-11-15 13:58 ` Chris Wilson
0 siblings, 0 replies; 28+ messages in thread
From: Chris Wilson @ 2019-11-15 13:58 UTC (permalink / raw)
To: Abdiel Janulgue, intel-gfx; +Cc: Matthew Auld
Quoting Abdiel Janulgue (2019-11-15 11:45:49)
> +static vm_fault_t i915_gem_fault_cpu(struct vm_fault *vmf)
> +{
> + struct vm_area_struct *area = vmf->vma;
> + struct i915_mmap_offset *priv = area->vm_private_data;
> + struct drm_i915_gem_object *obj = priv->obj;
> + vm_fault_t vmf_ret;
> + unsigned long i, size = area->vm_end - area->vm_start;
> + bool write = area->vm_flags & VM_WRITE;
> + int ret;
> +
> + if (!i915_gem_object_has_struct_page(obj))
> + return VM_FAULT_SIGBUS;
> +
> + /* Sanity check that we allow writing into this object */
> + if (i915_gem_object_is_readonly(obj) && write)
> + return VM_FAULT_SIGBUS;
> +
> + ret = i915_gem_object_pin_pages(obj);
> + if (ret)
> + return i915_error_to_vmf_fault(ret);
> +
It is probably a good idea to mention here how we revoke the PTE.
/* PTEs are revoked in obj->ops->put_pages() */
> + for (i = 0; i < size >> PAGE_SHIFT; i++) {
> + struct page *page = i915_gem_object_get_page(obj, i);
> +
> + vmf_ret = vmf_insert_pfn(area,
> + (unsigned long)area->vm_start + i * PAGE_SIZE,
> + page_to_pfn(page));
> + if (vmf_ret != VM_FAULT_NOPAGE)
> + break;
> + }
> +
> + i915_gem_object_unpin_pages(obj);
> +
> + return vmf_ret;
With the revoke + selftests in place, this looks correct.
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
There might be room for fine tuning (not pin/mapping the whole object),
but that has been a concern for a long time and remains a concern :)
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [Intel-gfx] [PATCH 4/4] drm/i915: Add cpu fault handler for mmap_offset
@ 2019-11-15 13:58 ` Chris Wilson
0 siblings, 0 replies; 28+ messages in thread
From: Chris Wilson @ 2019-11-15 13:58 UTC (permalink / raw)
To: Abdiel Janulgue, intel-gfx; +Cc: Matthew Auld
Quoting Abdiel Janulgue (2019-11-15 11:45:49)
> +static vm_fault_t i915_gem_fault_cpu(struct vm_fault *vmf)
> +{
> + struct vm_area_struct *area = vmf->vma;
> + struct i915_mmap_offset *priv = area->vm_private_data;
> + struct drm_i915_gem_object *obj = priv->obj;
> + vm_fault_t vmf_ret;
> + unsigned long i, size = area->vm_end - area->vm_start;
> + bool write = area->vm_flags & VM_WRITE;
> + int ret;
> +
> + if (!i915_gem_object_has_struct_page(obj))
> + return VM_FAULT_SIGBUS;
> +
> + /* Sanity check that we allow writing into this object */
> + if (i915_gem_object_is_readonly(obj) && write)
> + return VM_FAULT_SIGBUS;
> +
> + ret = i915_gem_object_pin_pages(obj);
> + if (ret)
> + return i915_error_to_vmf_fault(ret);
> +
It is probably a good idea to mention here how we revoke the PTE.
/* PTEs are revoked in obj->ops->put_pages() */
> + for (i = 0; i < size >> PAGE_SHIFT; i++) {
> + struct page *page = i915_gem_object_get_page(obj, i);
> +
> + vmf_ret = vmf_insert_pfn(area,
> + (unsigned long)area->vm_start + i * PAGE_SIZE,
> + page_to_pfn(page));
> + if (vmf_ret != VM_FAULT_NOPAGE)
> + break;
> + }
> +
> + i915_gem_object_unpin_pages(obj);
> +
> + return vmf_ret;
With the revoke + selftests in place, this looks correct.
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
There might be room for fine tuning (not pin/mapping the whole object),
but that has been a concern for a long time and remains a concern :)
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 1/4] drm/i915: Allow i915 to manage the vma offset nodes instead of drm core
@ 2019-11-15 14:17 ` Chris Wilson
0 siblings, 0 replies; 28+ messages in thread
From: Chris Wilson @ 2019-11-15 14:17 UTC (permalink / raw)
To: Abdiel Janulgue, intel-gfx; +Cc: Matthew Auld
Quoting Abdiel Janulgue (2019-11-15 11:45:46)
> -static int create_mmap_offset(struct drm_i915_gem_object *obj)
> +void i915_gem_object_release_mmap_offset(struct drm_i915_gem_object *obj)
> +{
> + struct i915_mmap_offset *mmo;
> +
> + mutex_lock(&obj->mmo_lock);
> + list_for_each_entry(mmo, &obj->mmap_offsets, offset) {
> + /* vma_node_unmap for GTT mmaps handled already in
> + * __i915_gem_object_release_mmap_gtt
> + */
> + if (mmo->mmap_type != I915_MMAP_TYPE_GTT)
Tempted to say always do it, but that would be a waste indeed.
> + drm_vma_node_unmap(&mmo->vma_node,
> + obj->base.dev->anon_inode->i_mapping);
> + }
> + mutex_unlock(&obj->mmo_lock);
> +}
> +void i915_mmap_offset_destroy(struct i915_mmap_offset *mmo, struct mutex *mutex)
> +{
> + if (mmo->file)
> + drm_vma_node_revoke(&mmo->vma_node, mmo->file);
Wait a sec.
The mmo is global, one per object per type. Not one per object per type
per client.
We shouldn't be associated with a single mmo->file. That is enough
address space magnification for a single process to be able to exhaust
the entire global address space...
How's this meant to work?
> @@ -118,6 +132,11 @@ struct drm_i915_gem_object {
> unsigned int userfault_count;
> struct list_head userfault_link;
>
> + /* Protects access to mmap offsets */
> + struct mutex mmo_lock;
> + struct list_head mmap_offsets;
> + bool readonly:1;
Go on, steal a bit from flags.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [Intel-gfx] [PATCH 1/4] drm/i915: Allow i915 to manage the vma offset nodes instead of drm core
@ 2019-11-15 14:17 ` Chris Wilson
0 siblings, 0 replies; 28+ messages in thread
From: Chris Wilson @ 2019-11-15 14:17 UTC (permalink / raw)
To: Abdiel Janulgue, intel-gfx; +Cc: Matthew Auld
Quoting Abdiel Janulgue (2019-11-15 11:45:46)
> -static int create_mmap_offset(struct drm_i915_gem_object *obj)
> +void i915_gem_object_release_mmap_offset(struct drm_i915_gem_object *obj)
> +{
> + struct i915_mmap_offset *mmo;
> +
> + mutex_lock(&obj->mmo_lock);
> + list_for_each_entry(mmo, &obj->mmap_offsets, offset) {
> + /* vma_node_unmap for GTT mmaps handled already in
> + * __i915_gem_object_release_mmap_gtt
> + */
> + if (mmo->mmap_type != I915_MMAP_TYPE_GTT)
Tempted to say always do it, but that would be a waste indeed.
> + drm_vma_node_unmap(&mmo->vma_node,
> + obj->base.dev->anon_inode->i_mapping);
> + }
> + mutex_unlock(&obj->mmo_lock);
> +}
> +void i915_mmap_offset_destroy(struct i915_mmap_offset *mmo, struct mutex *mutex)
> +{
> + if (mmo->file)
> + drm_vma_node_revoke(&mmo->vma_node, mmo->file);
Wait a sec.
The mmo is global, one per object per type. Not one per object per type
per client.
We shouldn't be associated with a single mmo->file. That is enough
address space magnification for a single process to be able to exhaust
the entire global address space...
How's this meant to work?
> @@ -118,6 +132,11 @@ struct drm_i915_gem_object {
> unsigned int userfault_count;
> struct list_head userfault_link;
>
> + /* Protects access to mmap offsets */
> + struct mutex mmo_lock;
> + struct list_head mmap_offsets;
> + bool readonly:1;
Go on, steal a bit from flags.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 28+ messages in thread
* ✗ Fi.CI.BAT: failure for series starting with [1/4] drm/i915: Allow i915 to manage the vma offset nodes instead of drm core
@ 2019-11-15 14:23 ` Patchwork
0 siblings, 0 replies; 28+ messages in thread
From: Patchwork @ 2019-11-15 14:23 UTC (permalink / raw)
To: Abdiel Janulgue; +Cc: intel-gfx
== Series Details ==
Series: series starting with [1/4] drm/i915: Allow i915 to manage the vma offset nodes instead of drm core
URL : https://patchwork.freedesktop.org/series/69522/
State : failure
== Summary ==
CI Bug Log - changes from CI_DRM_7352 -> Patchwork_15278
====================================================
Summary
-------
**FAILURE**
Serious unknown changes coming with Patchwork_15278 absolutely need to be
verified manually.
If you think the reported changes have nothing to do with the changes
introduced in Patchwork_15278, please notify your bug team to allow them
to document this new failure mode, which will reduce false positives in CI.
External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15278/index.html
Possible new issues
-------------------
Here are the unknown changes that may have been introduced in Patchwork_15278:
### IGT changes ###
#### Possible regressions ####
* igt@kms_pipe_crc_basic@hang-read-crc-pipe-a:
- fi-gdg-551: [PASS][1] -> [FAIL][2] +1 similar issue
[1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7352/fi-gdg-551/igt@kms_pipe_crc_basic@hang-read-crc-pipe-a.html
[2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15278/fi-gdg-551/igt@kms_pipe_crc_basic@hang-read-crc-pipe-a.html
* igt@kms_pipe_crc_basic@nonblocking-crc-pipe-a-frame-sequence:
- fi-ilk-650: [PASS][3] -> [FAIL][4] +6 similar issues
[3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7352/fi-ilk-650/igt@kms_pipe_crc_basic@nonblocking-crc-pipe-a-frame-sequence.html
[4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15278/fi-ilk-650/igt@kms_pipe_crc_basic@nonblocking-crc-pipe-a-frame-sequence.html
- fi-elk-e7500: [PASS][5] -> [FAIL][6] +6 similar issues
[5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7352/fi-elk-e7500/igt@kms_pipe_crc_basic@nonblocking-crc-pipe-a-frame-sequence.html
[6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15278/fi-elk-e7500/igt@kms_pipe_crc_basic@nonblocking-crc-pipe-a-frame-sequence.html
* igt@kms_pipe_crc_basic@read-crc-pipe-a:
- fi-bwr-2160: [PASS][7] -> [FAIL][8] +4 similar issues
[7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7352/fi-bwr-2160/igt@kms_pipe_crc_basic@read-crc-pipe-a.html
[8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15278/fi-bwr-2160/igt@kms_pipe_crc_basic@read-crc-pipe-a.html
* igt@kms_pipe_crc_basic@read-crc-pipe-b:
- fi-blb-e6850: [PASS][9] -> [FAIL][10] +6 similar issues
[9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7352/fi-blb-e6850/igt@kms_pipe_crc_basic@read-crc-pipe-b.html
[10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15278/fi-blb-e6850/igt@kms_pipe_crc_basic@read-crc-pipe-b.html
* igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a:
- fi-bsw-kefka: [PASS][11] -> [FAIL][12] +4 similar issues
[11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7352/fi-bsw-kefka/igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a.html
[12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15278/fi-bsw-kefka/igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a.html
#### Warnings ####
* igt@kms_chamelium@common-hpd-after-suspend:
- fi-kbl-7500u: [DMESG-WARN][13] ([fdo#102505] / [fdo#103558] / [fdo#105079] / [fdo#105602]) -> [FAIL][14]
[13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7352/fi-kbl-7500u/igt@kms_chamelium@common-hpd-after-suspend.html
[14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15278/fi-kbl-7500u/igt@kms_chamelium@common-hpd-after-suspend.html
Known issues
------------
Here are the changes found in Patchwork_15278 that come from known issues:
### IGT changes ###
#### Issues hit ####
* igt@i915_module_load@reload-no-display:
- fi-skl-lmem: [PASS][15] -> [DMESG-WARN][16] ([fdo#112261])
[15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7352/fi-skl-lmem/igt@i915_module_load@reload-no-display.html
[16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15278/fi-skl-lmem/igt@i915_module_load@reload-no-display.html
* igt@i915_selftest@live_blt:
- fi-bsw-n3050: [PASS][17] -> [DMESG-FAIL][18] ([fdo#112176])
[17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7352/fi-bsw-n3050/igt@i915_selftest@live_blt.html
[18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15278/fi-bsw-n3050/igt@i915_selftest@live_blt.html
* igt@i915_selftest@live_gem_contexts:
- fi-cfl-8700k: [PASS][19] -> [INCOMPLETE][20] ([fdo#111700])
[19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7352/fi-cfl-8700k/igt@i915_selftest@live_gem_contexts.html
[20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15278/fi-cfl-8700k/igt@i915_selftest@live_gem_contexts.html
* igt@kms_chamelium@hdmi-crc-fast:
- fi-kbl-7500u: [PASS][21] -> [FAIL][22] ([fdo#109635 ])
[21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7352/fi-kbl-7500u/igt@kms_chamelium@hdmi-crc-fast.html
[22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15278/fi-kbl-7500u/igt@kms_chamelium@hdmi-crc-fast.html
* igt@kms_chamelium@hdmi-hpd-fast:
- fi-kbl-7500u: [PASS][23] -> [FAIL][24] ([fdo#111407])
[23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7352/fi-kbl-7500u/igt@kms_chamelium@hdmi-hpd-fast.html
[24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15278/fi-kbl-7500u/igt@kms_chamelium@hdmi-hpd-fast.html
* igt@kms_flip@basic-flip-vs-wf_vblank:
- fi-skl-6770hq: [PASS][25] -> [DMESG-WARN][26] ([fdo#105541])
[25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7352/fi-skl-6770hq/igt@kms_flip@basic-flip-vs-wf_vblank.html
[26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15278/fi-skl-6770hq/igt@kms_flip@basic-flip-vs-wf_vblank.html
* igt@kms_pipe_crc_basic@nonblocking-crc-pipe-a-frame-sequence:
- fi-byt-n2820: [PASS][27] -> [FAIL][28] ([fdo#103191]) +5 similar issues
[27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7352/fi-byt-n2820/igt@kms_pipe_crc_basic@nonblocking-crc-pipe-a-frame-sequence.html
[28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15278/fi-byt-n2820/igt@kms_pipe_crc_basic@nonblocking-crc-pipe-a-frame-sequence.html
* igt@kms_pipe_crc_basic@read-crc-pipe-b:
- fi-byt-j1900: [PASS][29] -> [FAIL][30] ([fdo#103191]) +4 similar issues
[29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7352/fi-byt-j1900/igt@kms_pipe_crc_basic@read-crc-pipe-b.html
[30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15278/fi-byt-j1900/igt@kms_pipe_crc_basic@read-crc-pipe-b.html
* igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a:
- fi-glk-dsi: [PASS][31] -> [FAIL][32] ([fdo#103191]) +3 similar issues
[31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7352/fi-glk-dsi/igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a.html
[32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15278/fi-glk-dsi/igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a.html
* igt@kms_setmode@basic-clone-single-crtc:
- fi-skl-6770hq: [PASS][33] -> [WARN][34] ([fdo#112252])
[33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7352/fi-skl-6770hq/igt@kms_setmode@basic-clone-single-crtc.html
[34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15278/fi-skl-6770hq/igt@kms_setmode@basic-clone-single-crtc.html
#### Possible fixes ####
* igt@i915_module_load@reload-with-fault-injection:
- fi-skl-6770hq: [INCOMPLETE][35] -> [PASS][36]
[35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7352/fi-skl-6770hq/igt@i915_module_load@reload-with-fault-injection.html
[36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15278/fi-skl-6770hq/igt@i915_module_load@reload-with-fault-injection.html
* igt@i915_selftest@live_blt:
- fi-bsw-nick: [DMESG-FAIL][37] ([fdo#112176]) -> [PASS][38]
[37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7352/fi-bsw-nick/igt@i915_selftest@live_blt.html
[38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15278/fi-bsw-nick/igt@i915_selftest@live_blt.html
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
[fdo#102505]: https://bugs.freedesktop.org/show_bug.cgi?id=102505
[fdo#103191]: https://bugs.freedesktop.org/show_bug.cgi?id=103191
[fdo#103558]: https://bugs.freedesktop.org/show_bug.cgi?id=103558
[fdo#105079]: https://bugs.freedesktop.org/show_bug.cgi?id=105079
[fdo#105541]: https://bugs.freedesktop.org/show_bug.cgi?id=105541
[fdo#105602]: https://bugs.freedesktop.org/show_bug.cgi?id=105602
[fdo#109635 ]: https://bugs.freedesktop.org/show_bug.cgi?id=109635
[fdo#109964]: https://bugs.freedesktop.org/show_bug.cgi?id=109964
[fdo#111407]: https://bugs.freedesktop.org/show_bug.cgi?id=111407
[fdo#111700]: https://bugs.freedesktop.org/show_bug.cgi?id=111700
[fdo#112176]: https://bugs.freedesktop.org/show_bug.cgi?id=112176
[fdo#112252]: https://bugs.freedesktop.org/show_bug.cgi?id=112252
[fdo#112261]: https://bugs.freedesktop.org/show_bug.cgi?id=112261
[fdo#112298]: https://bugs.freedesktop.org/show_bug.cgi?id=112298
Participating hosts (53 -> 44)
------------------------------
Missing (9): fi-ilk-m540 fi-tgl-u fi-hsw-4200u fi-byt-squawks fi-icl-u2 fi-bsw-cyan fi-ctg-p8600 fi-byt-clapper fi-bdw-samus
Build changes
-------------
* CI: CI-20190529 -> None
* Linux: CI_DRM_7352 -> Patchwork_15278
CI-20190529: 20190529
CI_DRM_7352: 9d6a1b13121af95e05798c72a76bf00207816f0e @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_5288: ff4551e36cd8e573ceb1e450d17a12e3298dc04c @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_15278: 27a373a960f145240b543afdd6a2f933ed11af96 @ git://anongit.freedesktop.org/gfx-ci/linux
== Linux commits ==
27a373a960f1 drm/i915: Add cpu fault handler for mmap_offset
52a0eee7cfa2 drm/i915: cpu-map based dumb buffers
bb6e6f21d2f9 drm/i915: Introduce DRM_I915_GEM_MMAP_OFFSET
a0ff7bbb56ea drm/i915: Allow i915 to manage the vma offset nodes instead of drm core
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15278/index.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 28+ messages in thread
* [Intel-gfx] ✗ Fi.CI.BAT: failure for series starting with [1/4] drm/i915: Allow i915 to manage the vma offset nodes instead of drm core
@ 2019-11-15 14:23 ` Patchwork
0 siblings, 0 replies; 28+ messages in thread
From: Patchwork @ 2019-11-15 14:23 UTC (permalink / raw)
To: Abdiel Janulgue; +Cc: intel-gfx
== Series Details ==
Series: series starting with [1/4] drm/i915: Allow i915 to manage the vma offset nodes instead of drm core
URL : https://patchwork.freedesktop.org/series/69522/
State : failure
== Summary ==
CI Bug Log - changes from CI_DRM_7352 -> Patchwork_15278
====================================================
Summary
-------
**FAILURE**
Serious unknown changes coming with Patchwork_15278 absolutely need to be
verified manually.
If you think the reported changes have nothing to do with the changes
introduced in Patchwork_15278, please notify your bug team to allow them
to document this new failure mode, which will reduce false positives in CI.
External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15278/index.html
Possible new issues
-------------------
Here are the unknown changes that may have been introduced in Patchwork_15278:
### IGT changes ###
#### Possible regressions ####
* igt@kms_pipe_crc_basic@hang-read-crc-pipe-a:
- fi-gdg-551: [PASS][1] -> [FAIL][2] +1 similar issue
[1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7352/fi-gdg-551/igt@kms_pipe_crc_basic@hang-read-crc-pipe-a.html
[2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15278/fi-gdg-551/igt@kms_pipe_crc_basic@hang-read-crc-pipe-a.html
* igt@kms_pipe_crc_basic@nonblocking-crc-pipe-a-frame-sequence:
- fi-ilk-650: [PASS][3] -> [FAIL][4] +6 similar issues
[3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7352/fi-ilk-650/igt@kms_pipe_crc_basic@nonblocking-crc-pipe-a-frame-sequence.html
[4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15278/fi-ilk-650/igt@kms_pipe_crc_basic@nonblocking-crc-pipe-a-frame-sequence.html
- fi-elk-e7500: [PASS][5] -> [FAIL][6] +6 similar issues
[5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7352/fi-elk-e7500/igt@kms_pipe_crc_basic@nonblocking-crc-pipe-a-frame-sequence.html
[6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15278/fi-elk-e7500/igt@kms_pipe_crc_basic@nonblocking-crc-pipe-a-frame-sequence.html
* igt@kms_pipe_crc_basic@read-crc-pipe-a:
- fi-bwr-2160: [PASS][7] -> [FAIL][8] +4 similar issues
[7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7352/fi-bwr-2160/igt@kms_pipe_crc_basic@read-crc-pipe-a.html
[8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15278/fi-bwr-2160/igt@kms_pipe_crc_basic@read-crc-pipe-a.html
* igt@kms_pipe_crc_basic@read-crc-pipe-b:
- fi-blb-e6850: [PASS][9] -> [FAIL][10] +6 similar issues
[9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7352/fi-blb-e6850/igt@kms_pipe_crc_basic@read-crc-pipe-b.html
[10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15278/fi-blb-e6850/igt@kms_pipe_crc_basic@read-crc-pipe-b.html
* igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a:
- fi-bsw-kefka: [PASS][11] -> [FAIL][12] +4 similar issues
[11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7352/fi-bsw-kefka/igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a.html
[12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15278/fi-bsw-kefka/igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a.html
#### Warnings ####
* igt@kms_chamelium@common-hpd-after-suspend:
- fi-kbl-7500u: [DMESG-WARN][13] ([fdo#102505] / [fdo#103558] / [fdo#105079] / [fdo#105602]) -> [FAIL][14]
[13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7352/fi-kbl-7500u/igt@kms_chamelium@common-hpd-after-suspend.html
[14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15278/fi-kbl-7500u/igt@kms_chamelium@common-hpd-after-suspend.html
Known issues
------------
Here are the changes found in Patchwork_15278 that come from known issues:
### IGT changes ###
#### Issues hit ####
* igt@i915_module_load@reload-no-display:
- fi-skl-lmem: [PASS][15] -> [DMESG-WARN][16] ([fdo#112261])
[15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7352/fi-skl-lmem/igt@i915_module_load@reload-no-display.html
[16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15278/fi-skl-lmem/igt@i915_module_load@reload-no-display.html
* igt@i915_selftest@live_blt:
- fi-bsw-n3050: [PASS][17] -> [DMESG-FAIL][18] ([fdo#112176])
[17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7352/fi-bsw-n3050/igt@i915_selftest@live_blt.html
[18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15278/fi-bsw-n3050/igt@i915_selftest@live_blt.html
* igt@i915_selftest@live_gem_contexts:
- fi-cfl-8700k: [PASS][19] -> [INCOMPLETE][20] ([fdo#111700])
[19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7352/fi-cfl-8700k/igt@i915_selftest@live_gem_contexts.html
[20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15278/fi-cfl-8700k/igt@i915_selftest@live_gem_contexts.html
* igt@kms_chamelium@hdmi-crc-fast:
- fi-kbl-7500u: [PASS][21] -> [FAIL][22] ([fdo#109635 ])
[21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7352/fi-kbl-7500u/igt@kms_chamelium@hdmi-crc-fast.html
[22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15278/fi-kbl-7500u/igt@kms_chamelium@hdmi-crc-fast.html
* igt@kms_chamelium@hdmi-hpd-fast:
- fi-kbl-7500u: [PASS][23] -> [FAIL][24] ([fdo#111407])
[23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7352/fi-kbl-7500u/igt@kms_chamelium@hdmi-hpd-fast.html
[24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15278/fi-kbl-7500u/igt@kms_chamelium@hdmi-hpd-fast.html
* igt@kms_flip@basic-flip-vs-wf_vblank:
- fi-skl-6770hq: [PASS][25] -> [DMESG-WARN][26] ([fdo#105541])
[25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7352/fi-skl-6770hq/igt@kms_flip@basic-flip-vs-wf_vblank.html
[26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15278/fi-skl-6770hq/igt@kms_flip@basic-flip-vs-wf_vblank.html
* igt@kms_pipe_crc_basic@nonblocking-crc-pipe-a-frame-sequence:
- fi-byt-n2820: [PASS][27] -> [FAIL][28] ([fdo#103191]) +5 similar issues
[27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7352/fi-byt-n2820/igt@kms_pipe_crc_basic@nonblocking-crc-pipe-a-frame-sequence.html
[28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15278/fi-byt-n2820/igt@kms_pipe_crc_basic@nonblocking-crc-pipe-a-frame-sequence.html
* igt@kms_pipe_crc_basic@read-crc-pipe-b:
- fi-byt-j1900: [PASS][29] -> [FAIL][30] ([fdo#103191]) +4 similar issues
[29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7352/fi-byt-j1900/igt@kms_pipe_crc_basic@read-crc-pipe-b.html
[30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15278/fi-byt-j1900/igt@kms_pipe_crc_basic@read-crc-pipe-b.html
* igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a:
- fi-glk-dsi: [PASS][31] -> [FAIL][32] ([fdo#103191]) +3 similar issues
[31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7352/fi-glk-dsi/igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a.html
[32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15278/fi-glk-dsi/igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a.html
* igt@kms_setmode@basic-clone-single-crtc:
- fi-skl-6770hq: [PASS][33] -> [WARN][34] ([fdo#112252])
[33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7352/fi-skl-6770hq/igt@kms_setmode@basic-clone-single-crtc.html
[34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15278/fi-skl-6770hq/igt@kms_setmode@basic-clone-single-crtc.html
#### Possible fixes ####
* igt@i915_module_load@reload-with-fault-injection:
- fi-skl-6770hq: [INCOMPLETE][35] -> [PASS][36]
[35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7352/fi-skl-6770hq/igt@i915_module_load@reload-with-fault-injection.html
[36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15278/fi-skl-6770hq/igt@i915_module_load@reload-with-fault-injection.html
* igt@i915_selftest@live_blt:
- fi-bsw-nick: [DMESG-FAIL][37] ([fdo#112176]) -> [PASS][38]
[37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7352/fi-bsw-nick/igt@i915_selftest@live_blt.html
[38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15278/fi-bsw-nick/igt@i915_selftest@live_blt.html
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
[fdo#102505]: https://bugs.freedesktop.org/show_bug.cgi?id=102505
[fdo#103191]: https://bugs.freedesktop.org/show_bug.cgi?id=103191
[fdo#103558]: https://bugs.freedesktop.org/show_bug.cgi?id=103558
[fdo#105079]: https://bugs.freedesktop.org/show_bug.cgi?id=105079
[fdo#105541]: https://bugs.freedesktop.org/show_bug.cgi?id=105541
[fdo#105602]: https://bugs.freedesktop.org/show_bug.cgi?id=105602
[fdo#109635 ]: https://bugs.freedesktop.org/show_bug.cgi?id=109635
[fdo#109964]: https://bugs.freedesktop.org/show_bug.cgi?id=109964
[fdo#111407]: https://bugs.freedesktop.org/show_bug.cgi?id=111407
[fdo#111700]: https://bugs.freedesktop.org/show_bug.cgi?id=111700
[fdo#112176]: https://bugs.freedesktop.org/show_bug.cgi?id=112176
[fdo#112252]: https://bugs.freedesktop.org/show_bug.cgi?id=112252
[fdo#112261]: https://bugs.freedesktop.org/show_bug.cgi?id=112261
[fdo#112298]: https://bugs.freedesktop.org/show_bug.cgi?id=112298
Participating hosts (53 -> 44)
------------------------------
Missing (9): fi-ilk-m540 fi-tgl-u fi-hsw-4200u fi-byt-squawks fi-icl-u2 fi-bsw-cyan fi-ctg-p8600 fi-byt-clapper fi-bdw-samus
Build changes
-------------
* CI: CI-20190529 -> None
* Linux: CI_DRM_7352 -> Patchwork_15278
CI-20190529: 20190529
CI_DRM_7352: 9d6a1b13121af95e05798c72a76bf00207816f0e @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_5288: ff4551e36cd8e573ceb1e450d17a12e3298dc04c @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_15278: 27a373a960f145240b543afdd6a2f933ed11af96 @ git://anongit.freedesktop.org/gfx-ci/linux
== Linux commits ==
27a373a960f1 drm/i915: Add cpu fault handler for mmap_offset
52a0eee7cfa2 drm/i915: cpu-map based dumb buffers
bb6e6f21d2f9 drm/i915: Introduce DRM_I915_GEM_MMAP_OFFSET
a0ff7bbb56ea drm/i915: Allow i915 to manage the vma offset nodes instead of drm core
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15278/index.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 3/4] drm/i915: cpu-map based dumb buffers
@ 2019-11-15 14:26 ` Chris Wilson
0 siblings, 0 replies; 28+ messages in thread
From: Chris Wilson @ 2019-11-15 14:26 UTC (permalink / raw)
To: Abdiel Janulgue, intel-gfx; +Cc: Matthew Auld
Quoting Chris Wilson (2019-11-15 13:54:16)
> Quoting Abdiel Janulgue (2019-11-15 11:45:48)
> > Prefer CPU WC mmaps via our new mmap offset plumbing otherwise fall-
> > back to GTT mmaps when hw doesn't support PAT
> >
> > Signed-off-by: Abdiel Janulgue <abdiel.janulgue@linux.intel.com>
> > Cc: Matthew Auld <matthew.auld@intel.com>
> > ---
> > drivers/gpu/drm/i915/gem/i915_gem_mman.c | 18 ++++++++++++++++++
> > drivers/gpu/drm/i915/gem/i915_gem_mman.h | 2 ++
> > drivers/gpu/drm/i915/i915_drv.c | 1 +
> > 3 files changed, 21 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_mman.c b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
> > index d2ed8a463672..c1756e4f46b9 100644
> > --- a/drivers/gpu/drm/i915/gem/i915_gem_mman.c
> > +++ b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
> > @@ -540,6 +540,24 @@ __assign_gem_object_mmap_data(struct drm_file *file,
> > return ret;
> > }
> >
> > +int
> > +i915_gem_mmap_dumb(struct drm_file *file,
> > + struct drm_device *dev,
> > + u32 handle,
> > + u64 *offset)
> > +{
> > + enum i915_mmap_type mmap_type;
> > +
> > + if (boot_cpu_has(X86_FEATURE_PAT))
> > + mmap_type = I915_MMAP_TYPE_WC;
> > + else if (!i915_ggtt_has_aperture(&to_i915(dev)->ggtt))
> > + return -ENODEV;
> > + else
> > + mmap_type = I915_MMAP_TYPE_GTT;
> > +
> > + return __assign_gem_object_mmap_data(file, handle, mmap_type, offset);
>
> Looks ok. Just a few nagging doubts about potential existing misuse by
> userspace, such as are very using tiling on their dumb buffer, are they
> passing in a non-dumb handle?
Acked-by: Chris Wilson <chris@chris-wilson.co.uk>
Userspace review pending.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [Intel-gfx] [PATCH 3/4] drm/i915: cpu-map based dumb buffers
@ 2019-11-15 14:26 ` Chris Wilson
0 siblings, 0 replies; 28+ messages in thread
From: Chris Wilson @ 2019-11-15 14:26 UTC (permalink / raw)
To: Abdiel Janulgue, intel-gfx; +Cc: Matthew Auld
Quoting Chris Wilson (2019-11-15 13:54:16)
> Quoting Abdiel Janulgue (2019-11-15 11:45:48)
> > Prefer CPU WC mmaps via our new mmap offset plumbing otherwise fall-
> > back to GTT mmaps when hw doesn't support PAT
> >
> > Signed-off-by: Abdiel Janulgue <abdiel.janulgue@linux.intel.com>
> > Cc: Matthew Auld <matthew.auld@intel.com>
> > ---
> > drivers/gpu/drm/i915/gem/i915_gem_mman.c | 18 ++++++++++++++++++
> > drivers/gpu/drm/i915/gem/i915_gem_mman.h | 2 ++
> > drivers/gpu/drm/i915/i915_drv.c | 1 +
> > 3 files changed, 21 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_mman.c b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
> > index d2ed8a463672..c1756e4f46b9 100644
> > --- a/drivers/gpu/drm/i915/gem/i915_gem_mman.c
> > +++ b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
> > @@ -540,6 +540,24 @@ __assign_gem_object_mmap_data(struct drm_file *file,
> > return ret;
> > }
> >
> > +int
> > +i915_gem_mmap_dumb(struct drm_file *file,
> > + struct drm_device *dev,
> > + u32 handle,
> > + u64 *offset)
> > +{
> > + enum i915_mmap_type mmap_type;
> > +
> > + if (boot_cpu_has(X86_FEATURE_PAT))
> > + mmap_type = I915_MMAP_TYPE_WC;
> > + else if (!i915_ggtt_has_aperture(&to_i915(dev)->ggtt))
> > + return -ENODEV;
> > + else
> > + mmap_type = I915_MMAP_TYPE_GTT;
> > +
> > + return __assign_gem_object_mmap_data(file, handle, mmap_type, offset);
>
> Looks ok. Just a few nagging doubts about potential existing misuse by
> userspace, such as are very using tiling on their dumb buffer, are they
> passing in a non-dumb handle?
Acked-by: Chris Wilson <chris@chris-wilson.co.uk>
Userspace review pending.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 2/4] drm/i915: Introduce DRM_I915_GEM_MMAP_OFFSET
@ 2019-11-15 14:31 ` Chris Wilson
0 siblings, 0 replies; 28+ messages in thread
From: Chris Wilson @ 2019-11-15 14:31 UTC (permalink / raw)
To: Abdiel Janulgue, intel-gfx; +Cc: Matthew Auld
Quoting Abdiel Janulgue (2019-11-15 11:45:47)
> diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
> index 5400d7e057f1..e844c3a8d197 100644
> --- a/include/uapi/drm/i915_drm.h
> +++ b/include/uapi/drm/i915_drm.h
> @@ -395,6 +395,7 @@ typedef struct _drm_i915_sarea {
> #define DRM_IOCTL_I915_GEM_PWRITE DRM_IOW (DRM_COMMAND_BASE + DRM_I915_GEM_PWRITE, struct drm_i915_gem_pwrite)
> #define DRM_IOCTL_I915_GEM_MMAP DRM_IOWR(DRM_COMMAND_BASE + DRM_I915_GEM_MMAP, struct drm_i915_gem_mmap)
> #define DRM_IOCTL_I915_GEM_MMAP_GTT DRM_IOWR(DRM_COMMAND_BASE + DRM_I915_GEM_MMAP_GTT, struct drm_i915_gem_mmap_gtt)
> +#define DRM_IOCTL_I915_GEM_MMAP_OFFSET DRM_IOWR(DRM_COMMAND_BASE + DRM_I915_GEM_MMAP_GTT, struct drm_i915_gem_mmap_offset)
> #define DRM_IOCTL_I915_GEM_SET_DOMAIN DRM_IOW (DRM_COMMAND_BASE + DRM_I915_GEM_SET_DOMAIN, struct drm_i915_gem_set_domain)
> #define DRM_IOCTL_I915_GEM_SW_FINISH DRM_IOW (DRM_COMMAND_BASE + DRM_I915_GEM_SW_FINISH, struct drm_i915_gem_sw_finish)
> #define DRM_IOCTL_I915_GEM_SET_TILING DRM_IOWR (DRM_COMMAND_BASE + DRM_I915_GEM_SET_TILING, struct drm_i915_gem_set_tiling)
> @@ -793,6 +794,32 @@ struct drm_i915_gem_mmap_gtt {
> __u64 offset;
> };
>
> +struct drm_i915_gem_mmap_offset {
> + /** Handle for the object being mapped. */
> + __u32 handle;
> + __u32 pad;
> + /**
> + * Fake offset to use for subsequent mmap call
> + *
> + * This is a fixed-size type for 32/64 compatibility.
> + */
> + __u64 offset;
> +
> + /**
> + * Flags for extended behaviour.
> + *
> + * It is mandatory that either one of the MMAP_OFFSET flags
> + * should be passed here.
> + */
> + __u64 flags;
> +#define I915_MMAP_OFFSET_GTT 0
> +#define I915_MMAP_OFFSET_WC 1
> +#define I915_MMAP_OFFSET_WB 2
> +#define I915_MMAP_OFFSET_UC 3
The only question left here is should we
#define I915_MMAP_USE_EXTENSIONS (1 << 8)
from the start and add the dummy user extension handler to ease
adaption.
Otherwise the uABI looks correct; though it merits doing something like
struct drm_i915_gem_mmap_gtt arg = { valid state };
u64 redzone[16];
memset(redzone, 0xc5, sizeof(redzone));
igt_assert_eq(ioctl(I915_GEM_MMAP_GTT, &arg), 0);
as that would have failed with the earlier patch. A useful exercise to
work through to make sure you understand why.
Please write that igt asap.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [Intel-gfx] [PATCH 2/4] drm/i915: Introduce DRM_I915_GEM_MMAP_OFFSET
@ 2019-11-15 14:31 ` Chris Wilson
0 siblings, 0 replies; 28+ messages in thread
From: Chris Wilson @ 2019-11-15 14:31 UTC (permalink / raw)
To: Abdiel Janulgue, intel-gfx; +Cc: Matthew Auld
Quoting Abdiel Janulgue (2019-11-15 11:45:47)
> diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
> index 5400d7e057f1..e844c3a8d197 100644
> --- a/include/uapi/drm/i915_drm.h
> +++ b/include/uapi/drm/i915_drm.h
> @@ -395,6 +395,7 @@ typedef struct _drm_i915_sarea {
> #define DRM_IOCTL_I915_GEM_PWRITE DRM_IOW (DRM_COMMAND_BASE + DRM_I915_GEM_PWRITE, struct drm_i915_gem_pwrite)
> #define DRM_IOCTL_I915_GEM_MMAP DRM_IOWR(DRM_COMMAND_BASE + DRM_I915_GEM_MMAP, struct drm_i915_gem_mmap)
> #define DRM_IOCTL_I915_GEM_MMAP_GTT DRM_IOWR(DRM_COMMAND_BASE + DRM_I915_GEM_MMAP_GTT, struct drm_i915_gem_mmap_gtt)
> +#define DRM_IOCTL_I915_GEM_MMAP_OFFSET DRM_IOWR(DRM_COMMAND_BASE + DRM_I915_GEM_MMAP_GTT, struct drm_i915_gem_mmap_offset)
> #define DRM_IOCTL_I915_GEM_SET_DOMAIN DRM_IOW (DRM_COMMAND_BASE + DRM_I915_GEM_SET_DOMAIN, struct drm_i915_gem_set_domain)
> #define DRM_IOCTL_I915_GEM_SW_FINISH DRM_IOW (DRM_COMMAND_BASE + DRM_I915_GEM_SW_FINISH, struct drm_i915_gem_sw_finish)
> #define DRM_IOCTL_I915_GEM_SET_TILING DRM_IOWR (DRM_COMMAND_BASE + DRM_I915_GEM_SET_TILING, struct drm_i915_gem_set_tiling)
> @@ -793,6 +794,32 @@ struct drm_i915_gem_mmap_gtt {
> __u64 offset;
> };
>
> +struct drm_i915_gem_mmap_offset {
> + /** Handle for the object being mapped. */
> + __u32 handle;
> + __u32 pad;
> + /**
> + * Fake offset to use for subsequent mmap call
> + *
> + * This is a fixed-size type for 32/64 compatibility.
> + */
> + __u64 offset;
> +
> + /**
> + * Flags for extended behaviour.
> + *
> + * It is mandatory that either one of the MMAP_OFFSET flags
> + * should be passed here.
> + */
> + __u64 flags;
> +#define I915_MMAP_OFFSET_GTT 0
> +#define I915_MMAP_OFFSET_WC 1
> +#define I915_MMAP_OFFSET_WB 2
> +#define I915_MMAP_OFFSET_UC 3
The only question left here is should we
#define I915_MMAP_USE_EXTENSIONS (1 << 8)
from the start and add the dummy user extension handler to ease
adaption.
Otherwise the uABI looks correct; though it merits doing something like
struct drm_i915_gem_mmap_gtt arg = { valid state };
u64 redzone[16];
memset(redzone, 0xc5, sizeof(redzone));
igt_assert_eq(ioctl(I915_GEM_MMAP_GTT, &arg), 0);
as that would have failed with the earlier patch. A useful exercise to
work through to make sure you understand why.
Please write that igt asap.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH 1/4] drm/i915: Allow i915 to manage the vma offset nodes instead of drm core
@ 2019-11-19 11:37 Abdiel Janulgue
0 siblings, 0 replies; 28+ messages in thread
From: Abdiel Janulgue @ 2019-11-19 11:37 UTC (permalink / raw)
To: intel-gfx; +Cc: Matthew Auld
Have i915 replace the core drm_gem_mmap implementation to overcome its
limitation in having only a single mmap offset node per gem object.
This change allows us to have multiple mmap offsets per object and
enables a mmapping instance to use unique fault-handlers per user vma.
This allows i915 to store extra data within vma->vm_private_data and
assign the pagefault ops for each mmap instance allowing objects to use
multiple fault handlers depending on its backing storage.
v2:
- Fix race condition exposed by gem_mmap_gtt@close-race. Simplify
lifetime management of the mmap offset objects be ensuring it is
owned by the parent gem object instead of refcounting.
- Track mmo used by fencing to avoid locking when revoking mmaps
during GPU reset.
- Rebase.
v3:
- Simplify mmo tracking
v4:
- use vma->mmo in __i915_gem_object_release_mmap_gtt
v5:
- Revoke CPU mmaps on i915_gem_object_unbind() since unlike GTT
mmaps, they don't have bound i915_vma objects. Rebase.
v6: Minor tweaks, header re-org (Chris)
v7:
- Call drm_vma_node_revoke for the mmo with corresponding file when
releasing the gem handle instead of in the mmo's teardown.
- Add flag I915_BO_READONLY instead of obj->readonly.
Signed-off-by: Abdiel Janulgue <abdiel.janulgue@linux.intel.com>
Cc: Matthew Auld <matthew.auld@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
---
drivers/gpu/drm/i915/gem/i915_gem_domain.c | 3 +-
drivers/gpu/drm/i915/gem/i915_gem_mman.c | 241 +++++++++++++++---
drivers/gpu/drm/i915/gem/i915_gem_mman.h | 28 ++
drivers/gpu/drm/i915/gem/i915_gem_object.c | 18 ++
drivers/gpu/drm/i915/gem/i915_gem_object.h | 7 +-
.../gpu/drm/i915/gem/i915_gem_object_types.h | 19 ++
drivers/gpu/drm/i915/gem/i915_gem_pages.c | 3 +
drivers/gpu/drm/i915/gem/i915_gem_tiling.c | 1 +
.../drm/i915/gem/selftests/i915_gem_mman.c | 20 +-
drivers/gpu/drm/i915/gt/intel_reset.c | 7 +-
drivers/gpu/drm/i915/i915_drv.c | 11 +-
drivers/gpu/drm/i915/i915_drv.h | 3 -
drivers/gpu/drm/i915/i915_gem.c | 3 +-
drivers/gpu/drm/i915/i915_getparam.c | 1 +
drivers/gpu/drm/i915/i915_vma.c | 3 +-
drivers/gpu/drm/i915/i915_vma.h | 3 +
16 files changed, 313 insertions(+), 58 deletions(-)
create mode 100644 drivers/gpu/drm/i915/gem/i915_gem_mman.h
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_domain.c b/drivers/gpu/drm/i915/gem/i915_gem_domain.c
index e2af63af67ad..fd1c6f12b77a 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_domain.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_domain.c
@@ -13,6 +13,7 @@
#include "i915_gem_object.h"
#include "i915_vma.h"
#include "i915_gem_lmem.h"
+#include "i915_gem_mman.h"
static void __i915_gem_object_flush_for_display(struct drm_i915_gem_object *obj)
{
@@ -255,7 +256,7 @@ int i915_gem_object_set_cache_level(struct drm_i915_gem_object *obj,
}
if (obj->userfault_count)
- __i915_gem_object_release_mmap(obj);
+ __i915_gem_object_release_mmap_gtt(obj);
/*
* As we no longer need a fence for GTT access,
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_mman.c b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
index d60973603cc1..36fffb671601 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_mman.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
@@ -14,6 +14,7 @@
#include "i915_gem_gtt.h"
#include "i915_gem_ioctls.h"
#include "i915_gem_object.h"
+#include "i915_gem_mman.h"
#include "i915_trace.h"
#include "i915_vma.h"
@@ -219,7 +220,8 @@ vm_fault_t i915_gem_fault(struct vm_fault *vmf)
{
#define MIN_CHUNK_PAGES (SZ_1M >> PAGE_SHIFT)
struct vm_area_struct *area = vmf->vma;
- struct drm_i915_gem_object *obj = to_intel_bo(area->vm_private_data);
+ struct i915_mmap_offset *priv = area->vm_private_data;
+ struct drm_i915_gem_object *obj = priv->obj;
struct drm_device *dev = obj->base.dev;
struct drm_i915_private *i915 = to_i915(dev);
struct intel_runtime_pm *rpm = &i915->runtime_pm;
@@ -312,6 +314,9 @@ vm_fault_t i915_gem_fault(struct vm_fault *vmf)
list_add(&obj->userfault_link, &i915->ggtt.userfault_list);
mutex_unlock(&i915->ggtt.vm.mutex);
+ /* Track the mmo associated with the fenced vma */
+ vma->mmo = priv;
+
if (IS_ACTIVE(CONFIG_DRM_I915_USERFAULT_AUTOSUSPEND))
intel_wakeref_auto(&i915->ggtt.userfault_wakeref,
msecs_to_jiffies_timeout(CONFIG_DRM_I915_USERFAULT_AUTOSUSPEND));
@@ -358,28 +363,19 @@ vm_fault_t i915_gem_fault(struct vm_fault *vmf)
}
}
-void __i915_gem_object_release_mmap(struct drm_i915_gem_object *obj)
+void __i915_gem_object_release_mmap_gtt(struct drm_i915_gem_object *obj)
{
struct i915_vma *vma;
GEM_BUG_ON(!obj->userfault_count);
- obj->userfault_count = 0;
- list_del(&obj->userfault_link);
- drm_vma_node_unmap(&obj->base.vma_node,
- obj->base.dev->anon_inode->i_mapping);
-
for_each_ggtt_vma(vma, obj)
- i915_vma_unset_userfault(vma);
+ i915_vma_revoke_mmap(vma);
+
+ GEM_BUG_ON(obj->userfault_count);
}
/**
- * i915_gem_object_release_mmap - remove physical page mappings
- * @obj: obj in question
- *
- * Preserve the reservation of the mmapping with the DRM core code, but
- * relinquish ownership of the pages back to the system.
- *
* It is vital that we remove the page mapping if we have mapped a tiled
* object through the GTT and then lose the fence register due to
* resource pressure. Similarly if the object has been moved out of the
@@ -387,7 +383,7 @@ void __i915_gem_object_release_mmap(struct drm_i915_gem_object *obj)
* mapping will then trigger a page fault on the next user access, allowing
* fixup by i915_gem_fault().
*/
-void i915_gem_object_release_mmap(struct drm_i915_gem_object *obj)
+static void i915_gem_object_release_mmap_gtt(struct drm_i915_gem_object *obj)
{
struct drm_i915_private *i915 = to_i915(obj->base.dev);
intel_wakeref_t wakeref;
@@ -406,7 +402,7 @@ void i915_gem_object_release_mmap(struct drm_i915_gem_object *obj)
if (!obj->userfault_count)
goto out;
- __i915_gem_object_release_mmap(obj);
+ __i915_gem_object_release_mmap_gtt(obj);
/* Ensure that the CPU's PTE are revoked and there are not outstanding
* memory transactions from userspace before we return. The TLB
@@ -422,15 +418,63 @@ void i915_gem_object_release_mmap(struct drm_i915_gem_object *obj)
intel_runtime_pm_put(&i915->runtime_pm, wakeref);
}
-static int create_mmap_offset(struct drm_i915_gem_object *obj)
+void i915_gem_object_release_mmap_offset(struct drm_i915_gem_object *obj)
+{
+ struct i915_mmap_offset *mmo;
+
+ mutex_lock(&obj->mmo_lock);
+ list_for_each_entry(mmo, &obj->mmap_offsets, offset) {
+ /* vma_node_unmap for GTT mmaps handled already in
+ * __i915_gem_object_release_mmap_gtt
+ */
+ if (mmo->mmap_type != I915_MMAP_TYPE_GTT)
+ drm_vma_node_unmap(&mmo->vma_node,
+ obj->base.dev->anon_inode->i_mapping);
+ }
+ mutex_unlock(&obj->mmo_lock);
+}
+
+/**
+ * i915_gem_object_release_mmap - remove physical page mappings
+ * @obj: obj in question
+ *
+ * Preserve the reservation of the mmapping with the DRM core code, but
+ * relinquish ownership of the pages back to the system.
+ */
+void i915_gem_object_release_mmap(struct drm_i915_gem_object *obj)
+{
+ i915_gem_object_release_mmap_gtt(obj);
+ i915_gem_object_release_mmap_offset(obj);
+}
+
+static void init_mmap_offset(struct drm_i915_gem_object *obj,
+ struct i915_mmap_offset *mmo)
+{
+ mutex_lock(&obj->mmo_lock);
+ list_add(&mmo->offset, &obj->mmap_offsets);
+ mutex_unlock(&obj->mmo_lock);
+
+ mmo->obj = obj;
+ mmo->dev = obj->base.dev;
+}
+
+static int create_mmap_offset(struct drm_i915_gem_object *obj,
+ struct i915_mmap_offset *mmo)
{
struct drm_i915_private *i915 = to_i915(obj->base.dev);
struct intel_gt *gt = &i915->gt;
+ struct drm_device *dev = obj->base.dev;
int err;
- err = drm_gem_create_mmap_offset(&obj->base);
- if (likely(!err))
+ drm_vma_node_reset(&mmo->vma_node);
+ if (mmo->file)
+ drm_vma_node_allow(&mmo->vma_node, mmo->file);
+ err = drm_vma_offset_add(dev->vma_offset_manager, &mmo->vma_node,
+ obj->base.size / PAGE_SIZE);
+ if (likely(!err)) {
+ init_mmap_offset(obj, mmo);
return 0;
+ }
/* Attempt to reap some mmap space from dead objects */
err = intel_gt_retire_requests_timeout(gt, MAX_SCHEDULE_TIMEOUT);
@@ -438,34 +482,56 @@ static int create_mmap_offset(struct drm_i915_gem_object *obj)
return err;
i915_gem_drain_freed_objects(i915);
- return drm_gem_create_mmap_offset(&obj->base);
+ err = drm_vma_offset_add(dev->vma_offset_manager, &mmo->vma_node,
+ obj->base.size / PAGE_SIZE);
+ if (err)
+ return err;
+
+ init_mmap_offset(obj, mmo);
+ return 0;
}
-int
-i915_gem_mmap_gtt(struct drm_file *file,
- struct drm_device *dev,
- u32 handle,
- u64 *offset)
+static int
+__assign_gem_object_mmap_data(struct drm_file *file,
+ u32 handle,
+ enum i915_mmap_type mmap_type,
+ u64 *offset)
{
struct drm_i915_gem_object *obj;
+ struct i915_mmap_offset *mmo;
int ret;
- if (!i915_ggtt_has_aperture(&to_i915(dev)->ggtt))
- return -ENODEV;
-
obj = i915_gem_object_lookup(file, handle);
if (!obj)
return -ENOENT;
- if (i915_gem_object_never_bind_ggtt(obj)) {
+ if (mmap_type == I915_MMAP_TYPE_GTT &&
+ i915_gem_object_never_bind_ggtt(obj)) {
ret = -ENODEV;
goto out;
}
- ret = create_mmap_offset(obj);
- if (ret == 0)
- *offset = drm_vma_node_offset_addr(&obj->base.vma_node);
+ if (mmap_type != I915_MMAP_TYPE_GTT &&
+ !i915_gem_object_has_struct_page(obj)) {
+ ret = -ENODEV;
+ goto out;
+ }
+
+ mmo = kzalloc(sizeof(*mmo), GFP_KERNEL);
+ if (!mmo) {
+ ret = -ENOMEM;
+ goto out;
+ }
+
+ mmo->file = file;
+ ret = create_mmap_offset(obj, mmo);
+ if (ret) {
+ kfree(mmo);
+ goto out;
+ }
+ mmo->mmap_type = mmap_type;
+ *offset = drm_vma_node_offset_addr(&mmo->vma_node);
out:
i915_gem_object_put(obj);
return ret;
@@ -492,7 +558,116 @@ i915_gem_mmap_gtt_ioctl(struct drm_device *dev, void *data,
{
struct drm_i915_gem_mmap_gtt *args = data;
- return i915_gem_mmap_gtt(file, dev, args->handle, &args->offset);
+ return __assign_gem_object_mmap_data(file, args->handle,
+ I915_MMAP_TYPE_GTT,
+ &args->offset);
+}
+
+void i915_mmap_offset_destroy(struct i915_mmap_offset *mmo, struct mutex *mutex)
+{
+ drm_vma_offset_remove(mmo->dev->vma_offset_manager, &mmo->vma_node);
+
+ mutex_lock(mutex);
+ list_del(&mmo->offset);
+ mutex_unlock(mutex);
+
+ kfree(mmo);
+}
+
+static void i915_gem_vm_open(struct vm_area_struct *vma)
+{
+ struct i915_mmap_offset *mmo = vma->vm_private_data;
+ struct drm_i915_gem_object *obj = mmo->obj;
+
+ GEM_BUG_ON(!obj);
+ i915_gem_object_get(obj);
+}
+
+static void i915_gem_vm_close(struct vm_area_struct *vma)
+{
+ struct i915_mmap_offset *mmo = vma->vm_private_data;
+ struct drm_i915_gem_object *obj = mmo->obj;
+
+ GEM_BUG_ON(!obj);
+ i915_gem_object_put(obj);
+}
+
+static const struct vm_operations_struct i915_gem_gtt_vm_ops = {
+ .fault = i915_gem_fault,
+ .open = i915_gem_vm_open,
+ .close = i915_gem_vm_close,
+};
+
+/* This overcomes the limitation in drm_gem_mmap's assignment of a
+ * drm_gem_object as the vma->vm_private_data. Since we need to
+ * be able to resolve multiple mmap offsets which could be tied
+ * to a single gem object.
+ */
+int i915_gem_mmap(struct file *filp, struct vm_area_struct *vma)
+{
+ struct drm_vma_offset_node *node;
+ struct drm_file *priv = filp->private_data;
+ struct drm_device *dev = priv->minor->dev;
+ struct i915_mmap_offset *mmo = NULL;
+ struct drm_gem_object *obj = NULL;
+
+ if (drm_dev_is_unplugged(dev))
+ return -ENODEV;
+
+ drm_vma_offset_lock_lookup(dev->vma_offset_manager);
+ node = drm_vma_offset_exact_lookup_locked(dev->vma_offset_manager,
+ vma->vm_pgoff,
+ vma_pages(vma));
+ if (likely(node)) {
+ mmo = container_of(node, struct i915_mmap_offset,
+ vma_node);
+ /*
+ * In our dependency chain, the drm_vma_offset_node
+ * depends on the validity of the mmo, which depends on
+ * the gem object. However the only reference we have
+ * at this point is the mmo (as the parent of the node).
+ * Try to check if the gem object was at least cleared.
+ */
+ if (!mmo || !mmo->obj) {
+ drm_vma_offset_unlock_lookup(dev->vma_offset_manager);
+ return -EINVAL;
+ }
+ /*
+ * Skip 0-refcnted objects as it is in the process of being
+ * destroyed and will be invalid when the vma manager lock
+ * is released.
+ */
+ obj = &mmo->obj->base;
+ if (!kref_get_unless_zero(&obj->refcount))
+ obj = NULL;
+
+ }
+ drm_vma_offset_unlock_lookup(dev->vma_offset_manager);
+
+ if (!obj)
+ return -EINVAL;
+
+ if (!drm_vma_node_is_allowed(node, priv)) {
+ drm_gem_object_put_unlocked(obj);
+ return -EACCES;
+ }
+
+ if (i915_gem_object_is_readonly(to_intel_bo(obj))) {
+ if (vma->vm_flags & VM_WRITE) {
+ drm_gem_object_put_unlocked(obj);
+ return -EINVAL;
+ }
+ vma->vm_flags &= ~VM_MAYWRITE;
+ }
+
+ vma->vm_flags |= VM_PFNMAP | VM_DONTEXPAND | VM_DONTDUMP;
+ vma->vm_page_prot = pgprot_writecombine(vm_get_page_prot(vma->vm_flags));
+ vma->vm_page_prot = pgprot_decrypted(vma->vm_page_prot);
+ vma->vm_private_data = mmo;
+
+ vma->vm_ops = &i915_gem_gtt_vm_ops;
+
+ return 0;
}
#if IS_ENABLED(CONFIG_DRM_I915_SELFTEST)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_mman.h b/drivers/gpu/drm/i915/gem/i915_gem_mman.h
new file mode 100644
index 000000000000..25a3c4d6cd65
--- /dev/null
+++ b/drivers/gpu/drm/i915/gem/i915_gem_mman.h
@@ -0,0 +1,28 @@
+/*
+ * SPDX-License-Identifier: MIT
+ *
+ * Copyright © 2019 Intel Corporation
+ */
+
+#ifndef __I915_GEM_MMAN_H__
+#define __I915_GEM_MMAN_H__
+
+#include <linux/mm_types.h>
+#include <linux/types.h>
+
+struct drm_device;
+struct drm_file;
+struct drm_i915_gem_object;
+struct file;
+struct i915_mmap_offset;
+struct mutex;
+
+int i915_gem_mmap(struct file *filp, struct vm_area_struct *vma);
+vm_fault_t i915_gem_fault(struct vm_fault *vmf);
+void i915_mmap_offset_destroy(struct i915_mmap_offset *mmo, struct mutex *mutex);
+
+void __i915_gem_object_release_mmap_gtt(struct drm_i915_gem_object *obj);
+void i915_gem_object_release_mmap(struct drm_i915_gem_object *obj);
+void i915_gem_object_release_mmap_offset(struct drm_i915_gem_object *obj);
+
+#endif
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.c b/drivers/gpu/drm/i915/gem/i915_gem_object.c
index db103d3c8760..44cb5a2b140d 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_object.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_object.c
@@ -29,6 +29,7 @@
#include "i915_drv.h"
#include "i915_gem_clflush.h"
#include "i915_gem_context.h"
+#include "i915_gem_mman.h"
#include "i915_gem_object.h"
#include "i915_globals.h"
#include "i915_trace.h"
@@ -61,6 +62,9 @@ void i915_gem_object_init(struct drm_i915_gem_object *obj,
INIT_LIST_HEAD(&obj->lut_list);
+ mutex_init(&obj->mmo_lock);
+ INIT_LIST_HEAD(&obj->mmap_offsets);
+
init_rcu_head(&obj->rcu);
obj->ops = ops;
@@ -97,6 +101,7 @@ void i915_gem_close_object(struct drm_gem_object *gem, struct drm_file *file)
struct drm_i915_gem_object *obj = to_intel_bo(gem);
struct drm_i915_file_private *fpriv = file->driver_priv;
struct i915_lut_handle *lut, *ln;
+ struct i915_mmap_offset *mmo, *on;
LIST_HEAD(close);
i915_gem_object_lock(obj);
@@ -111,6 +116,9 @@ void i915_gem_close_object(struct drm_gem_object *gem, struct drm_file *file)
}
i915_gem_object_unlock(obj);
+ list_for_each_entry_safe(mmo, on, &obj->mmap_offsets, offset)
+ drm_vma_node_revoke(&mmo->vma_node, file);
+
list_for_each_entry_safe(lut, ln, &close, obj_link) {
struct i915_gem_context *ctx = lut->ctx;
struct i915_vma *vma;
@@ -158,6 +166,8 @@ static void __i915_gem_free_objects(struct drm_i915_private *i915,
wakeref = intel_runtime_pm_get(&i915->runtime_pm);
llist_for_each_entry_safe(obj, on, freed, freed) {
+ struct i915_mmap_offset *mmo, *mn;
+
trace_i915_gem_object_destroy(obj);
if (!list_empty(&obj->vma.list)) {
@@ -183,6 +193,14 @@ static void __i915_gem_free_objects(struct drm_i915_private *i915,
spin_unlock(&obj->vma.lock);
}
+ i915_gem_object_release_mmap(obj);
+
+ list_for_each_entry_safe(mmo, mn, &obj->mmap_offsets, offset) {
+ mmo->obj = NULL;
+ i915_mmap_offset_destroy(mmo, &obj->mmo_lock);
+ }
+
+ GEM_BUG_ON(!list_empty(&obj->mmap_offsets));
GEM_BUG_ON(atomic_read(&obj->bind_count));
GEM_BUG_ON(obj->userfault_count);
GEM_BUG_ON(!list_empty(&obj->lut_list));
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.h b/drivers/gpu/drm/i915/gem/i915_gem_object.h
index e5750d506cc9..a1eb7c0b23ac 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_object.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_object.h
@@ -132,13 +132,13 @@ void i915_gem_object_unlock_fence(struct drm_i915_gem_object *obj,
static inline void
i915_gem_object_set_readonly(struct drm_i915_gem_object *obj)
{
- obj->base.vma_node.readonly = true;
+ obj->flags |= I915_BO_READONLY;
}
static inline bool
i915_gem_object_is_readonly(const struct drm_i915_gem_object *obj)
{
- return obj->base.vma_node.readonly;
+ return obj->flags & I915_BO_READONLY;
}
static inline bool
@@ -387,9 +387,6 @@ static inline void i915_gem_object_unpin_map(struct drm_i915_gem_object *obj)
i915_gem_object_unpin_pages(obj);
}
-void __i915_gem_object_release_mmap(struct drm_i915_gem_object *obj);
-void i915_gem_object_release_mmap(struct drm_i915_gem_object *obj);
-
void
i915_gem_object_flush_write_domain(struct drm_i915_gem_object *obj,
unsigned int flush_domains);
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object_types.h b/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
index 15f8297dc34e..8ff0834a1d5c 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
@@ -63,6 +63,20 @@ struct drm_i915_gem_object_ops {
void (*release)(struct drm_i915_gem_object *obj);
};
+enum i915_mmap_type {
+ I915_MMAP_TYPE_GTT = 0,
+};
+
+struct i915_mmap_offset {
+ struct drm_device *dev;
+ struct drm_vma_offset_node vma_node;
+ struct drm_i915_gem_object *obj;
+ struct drm_file *file;
+ enum i915_mmap_type mmap_type;
+
+ struct list_head offset;
+};
+
struct drm_i915_gem_object {
struct drm_gem_object base;
@@ -118,12 +132,17 @@ struct drm_i915_gem_object {
unsigned int userfault_count;
struct list_head userfault_link;
+ /* Protects access to mmap offsets */
+ struct mutex mmo_lock;
+ struct list_head mmap_offsets;
+
I915_SELFTEST_DECLARE(struct list_head st_link);
unsigned long flags;
#define I915_BO_ALLOC_CONTIGUOUS BIT(0)
#define I915_BO_ALLOC_VOLATILE BIT(1)
#define I915_BO_ALLOC_FLAGS (I915_BO_ALLOC_CONTIGUOUS | I915_BO_ALLOC_VOLATILE)
+#define I915_BO_READONLY BIT(2)
/*
* Is the object to be mapped as read-only to the GPU
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_pages.c b/drivers/gpu/drm/i915/gem/i915_gem_pages.c
index f402c2c415c2..75197ca696a8 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_pages.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_pages.c
@@ -8,6 +8,7 @@
#include "i915_gem_object.h"
#include "i915_scatterlist.h"
#include "i915_gem_lmem.h"
+#include "i915_gem_mman.h"
void __i915_gem_object_set_pages(struct drm_i915_gem_object *obj,
struct sg_table *pages,
@@ -207,6 +208,8 @@ int __i915_gem_object_put_pages(struct drm_i915_gem_object *obj)
goto unlock;
}
+ i915_gem_object_release_mmap_offset(obj);
+
/*
* ->put_pages might need to allocate memory for the bit17 swizzle
* array, hence protect them from being reaped by removing them from gtt
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_tiling.c b/drivers/gpu/drm/i915/gem/i915_gem_tiling.c
index 1fa592d82af5..6c7825a2dc2a 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_tiling.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_tiling.c
@@ -11,6 +11,7 @@
#include "i915_drv.h"
#include "i915_gem.h"
#include "i915_gem_ioctls.h"
+#include "i915_gem_mman.h"
#include "i915_gem_object.h"
/**
diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c b/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c
index 9f1a69027a04..6c2333c61469 100644
--- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c
+++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c
@@ -564,15 +564,20 @@ static bool assert_mmap_offset(struct drm_i915_private *i915,
int expected)
{
struct drm_i915_gem_object *obj;
+ /* Ownership transferred to parent gem object in create_mmap_offset */
+ struct i915_mmap_offset *mmo = kzalloc(sizeof(*mmo), GFP_KERNEL);
int err;
obj = i915_gem_object_create_internal(i915, size);
if (IS_ERR(obj))
return PTR_ERR(obj);
- err = create_mmap_offset(obj);
+ err = create_mmap_offset(obj, mmo);
+ if (err)
+ kfree(mmo);
i915_gem_object_put(obj);
+
return err == expected;
}
@@ -608,6 +613,8 @@ static int igt_mmap_offset_exhaustion(void *arg)
struct drm_mm *mm = &i915->drm.vma_offset_manager->vm_addr_space_mm;
struct drm_i915_gem_object *obj;
struct drm_mm_node *hole, *next;
+ /* Ownership transferred to parent gem object in create_mmap_offset */
+ struct i915_mmap_offset *mmo = kzalloc(sizeof(*mmo), GFP_KERNEL);
int loop, err;
/* Disable background reaper */
@@ -672,9 +679,10 @@ static int igt_mmap_offset_exhaustion(void *arg)
goto out;
}
- err = create_mmap_offset(obj);
+ err = create_mmap_offset(obj, mmo);
if (err) {
pr_err("Unable to insert object into reclaimed hole\n");
+ kfree(mmo);
goto err_obj;
}
@@ -728,6 +736,8 @@ static int igt_mmap_gtt(void *arg)
struct drm_i915_private *i915 = arg;
struct drm_i915_gem_object *obj;
struct vm_area_struct *area;
+ /* Ownership transferred to parent gem object in create_mmap_offset */
+ struct i915_mmap_offset *mmo = kzalloc(sizeof(*mmo), GFP_KERNEL);
unsigned long addr;
void *vaddr;
int err, i;
@@ -748,7 +758,7 @@ static int igt_mmap_gtt(void *arg)
i915_gem_object_flush_map(obj);
i915_gem_object_unpin_map(obj);
- err = create_mmap_offset(obj);
+ err = create_mmap_offset(obj, mmo);
if (err)
goto out;
@@ -876,6 +886,8 @@ static int igt_mmap_gtt_revoke(void *arg)
{
struct drm_i915_private *i915 = arg;
struct drm_i915_gem_object *obj;
+ /* Ownership transferred to parent gem object in create_mmap_offset */
+ struct i915_mmap_offset *mmo = kzalloc(sizeof(*mmo), GFP_KERNEL);
unsigned long addr;
int err;
@@ -886,7 +898,7 @@ static int igt_mmap_gtt_revoke(void *arg)
if (IS_ERR(obj))
return PTR_ERR(obj);
- err = create_mmap_offset(obj);
+ err = create_mmap_offset(obj, mmo);
if (err)
goto out;
diff --git a/drivers/gpu/drm/i915/gt/intel_reset.c b/drivers/gpu/drm/i915/gt/intel_reset.c
index b7007cd78c6f..777eaa483676 100644
--- a/drivers/gpu/drm/i915/gt/intel_reset.c
+++ b/drivers/gpu/drm/i915/gt/intel_reset.c
@@ -678,8 +678,13 @@ static void revoke_mmaps(struct intel_gt *gt)
continue;
GEM_BUG_ON(vma->fence != >->ggtt->fence_regs[i]);
- node = &vma->obj->base.vma_node;
+
+ if (!vma->mmo)
+ continue;
+
+ node = &vma->mmo->vma_node;
vma_offset = vma->ggtt_view.partial.offset << PAGE_SHIFT;
+
unmap_mapping_range(gt->i915->drm.anon_inode->i_mapping,
drm_vma_node_offset_addr(node) + vma_offset,
vma->size,
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 8d7049792d62..3e129bcf8a12 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -61,6 +61,7 @@
#include "gem/i915_gem_context.h"
#include "gem/i915_gem_ioctls.h"
+#include "gem/i915_gem_mman.h"
#include "gt/intel_gt.h"
#include "gt/intel_gt_pm.h"
#include "gt/intel_rc6.h"
@@ -2662,18 +2663,12 @@ const struct dev_pm_ops i915_pm_ops = {
.runtime_resume = intel_runtime_resume,
};
-static const struct vm_operations_struct i915_gem_vm_ops = {
- .fault = i915_gem_fault,
- .open = drm_gem_vm_open,
- .close = drm_gem_vm_close,
-};
-
static const struct file_operations i915_driver_fops = {
.owner = THIS_MODULE,
.open = drm_open,
.release = drm_release,
.unlocked_ioctl = drm_ioctl,
- .mmap = drm_gem_mmap,
+ .mmap = i915_gem_mmap,
.poll = drm_poll,
.read = drm_read,
.compat_ioctl = i915_compat_ioctl,
@@ -2762,7 +2757,6 @@ static struct drm_driver driver = {
.gem_close_object = i915_gem_close_object,
.gem_free_object_unlocked = i915_gem_free_object,
- .gem_vm_ops = &i915_gem_vm_ops,
.prime_handle_to_fd = drm_gem_prime_handle_to_fd,
.prime_fd_to_handle = drm_gem_prime_fd_to_handle,
@@ -2773,7 +2767,6 @@ static struct drm_driver driver = {
.get_scanout_position = i915_get_crtc_scanoutpos,
.dumb_create = i915_gem_dumb_create,
- .dumb_map_offset = i915_gem_mmap_gtt,
.ioctls = i915_ioctls,
.num_ioctls = ARRAY_SIZE(i915_ioctls),
.fops = &i915_driver_fops,
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index bbf4dfdfa8ba..a36c2778c09c 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1853,8 +1853,6 @@ i915_mutex_lock_interruptible(struct drm_device *dev)
int i915_gem_dumb_create(struct drm_file *file_priv,
struct drm_device *dev,
struct drm_mode_create_dumb *args);
-int i915_gem_mmap_gtt(struct drm_file *file_priv, struct drm_device *dev,
- u32 handle, u64 *offset);
int i915_gem_mmap_gtt_version(void);
int __must_check i915_gem_set_global_seqno(struct drm_device *dev, u32 seqno);
@@ -1878,7 +1876,6 @@ void i915_gem_driver_release(struct drm_i915_private *dev_priv);
void i915_gem_suspend(struct drm_i915_private *dev_priv);
void i915_gem_suspend_late(struct drm_i915_private *dev_priv);
void i915_gem_resume(struct drm_i915_private *dev_priv);
-vm_fault_t i915_gem_fault(struct vm_fault *vmf);
int i915_gem_open(struct drm_i915_private *i915, struct drm_file *file);
void i915_gem_release(struct drm_device *dev, struct drm_file *file);
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 1ba5f26700b0..ca4d4e012a62 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -44,6 +44,7 @@
#include "gem/i915_gem_clflush.h"
#include "gem/i915_gem_context.h"
#include "gem/i915_gem_ioctls.h"
+#include "gem/i915_gem_mman.h"
#include "gem/i915_gem_pm.h"
#include "gt/intel_engine_user.h"
#include "gt/intel_gt.h"
@@ -854,7 +855,7 @@ void i915_gem_runtime_suspend(struct drm_i915_private *i915)
list_for_each_entry_safe(obj, on,
&i915->ggtt.userfault_list, userfault_link)
- __i915_gem_object_release_mmap(obj);
+ __i915_gem_object_release_mmap_gtt(obj);
/*
* The fence will be lost when the device powers down. If any were
diff --git a/drivers/gpu/drm/i915/i915_getparam.c b/drivers/gpu/drm/i915/i915_getparam.c
index cf8a8c3ef047..54fce81d5724 100644
--- a/drivers/gpu/drm/i915/i915_getparam.c
+++ b/drivers/gpu/drm/i915/i915_getparam.c
@@ -2,6 +2,7 @@
* SPDX-License-Identifier: MIT
*/
+#include "gem/i915_gem_mman.h"
#include "gt/intel_engine_user.h"
#include "i915_drv.h"
diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c
index e5512f26e20a..5108dcb2e5b1 100644
--- a/drivers/gpu/drm/i915/i915_vma.c
+++ b/drivers/gpu/drm/i915/i915_vma.c
@@ -1054,7 +1054,7 @@ static void __i915_vma_iounmap(struct i915_vma *vma)
void i915_vma_revoke_mmap(struct i915_vma *vma)
{
- struct drm_vma_offset_node *node = &vma->obj->base.vma_node;
+ struct drm_vma_offset_node *node;
u64 vma_offset;
lockdep_assert_held(&vma->vm->mutex);
@@ -1065,6 +1065,7 @@ void i915_vma_revoke_mmap(struct i915_vma *vma)
GEM_BUG_ON(!i915_vma_is_map_and_fenceable(vma));
GEM_BUG_ON(!vma->obj->userfault_count);
+ node = &vma->mmo->vma_node;
vma_offset = vma->ggtt_view.partial.offset << PAGE_SHIFT;
unmap_mapping_range(vma->vm->i915->drm.anon_inode->i_mapping,
drm_vma_node_offset_addr(node) + vma_offset,
diff --git a/drivers/gpu/drm/i915/i915_vma.h b/drivers/gpu/drm/i915/i915_vma.h
index 465932813bc5..f09f4f513c41 100644
--- a/drivers/gpu/drm/i915/i915_vma.h
+++ b/drivers/gpu/drm/i915/i915_vma.h
@@ -63,6 +63,9 @@ struct i915_vma {
u64 display_alignment;
struct i915_page_sizes page_sizes;
+ /* mmap-offset associated with fencing for this vma */
+ struct i915_mmap_offset *mmo;
+
u32 fence_size;
u32 fence_alignment;
--
2.23.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 1/4] drm/i915: Allow i915 to manage the vma offset nodes instead of drm core
@ 2019-11-14 19:09 Abdiel Janulgue
0 siblings, 0 replies; 28+ messages in thread
From: Abdiel Janulgue @ 2019-11-14 19:09 UTC (permalink / raw)
To: intel-gfx; +Cc: Matthew Auld
Have i915 replace the core drm_gem_mmap implementation to overcome its
limitation in having only a single mmap offset node per gem object.
This change allows us to have multiple mmap offsets per object and
enables a mmapping instance to use unique fault-handlers per user vma.
This allows i915 to store extra data within vma->vm_private_data and
assign the pagefault ops for each mmap instance allowing objects to use
multiple fault handlers depending on its backing storage.
v2:
- Fix race condition exposed by gem_mmap_gtt@close-race. Simplify
lifetime management of the mmap offset objects be ensuring it is
owned by the parent gem object instead of refcounting.
- Track mmo used by fencing to Avoid locking when revoking mmaps
during GPU reset.
- Rebase.
v3:
- Simplify mmo tracking
v4:
- use vma->mmo in __i915_gem_object_release_mmap_gtt
v5:
- Revoke CPU mmaps on i915_gem_object_unbind() since unlike GTT
mmaps, they don't have bound i915_vmas objects. Rebase.
Signed-off-by: Abdiel Janulgue <abdiel.janulgue@linux.intel.com>
Cc: Matthew Auld <matthew.auld@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
---
drivers/gpu/drm/i915/gem/i915_gem_domain.c | 2 +-
drivers/gpu/drm/i915/gem/i915_gem_mman.c | 235 +++++++++++++++---
drivers/gpu/drm/i915/gem/i915_gem_object.c | 13 +
drivers/gpu/drm/i915/gem/i915_gem_object.h | 8 +-
.../gpu/drm/i915/gem/i915_gem_object_types.h | 19 ++
.../drm/i915/gem/selftests/i915_gem_mman.c | 12 +-
drivers/gpu/drm/i915/gt/intel_reset.c | 7 +-
drivers/gpu/drm/i915/i915_drv.c | 10 +-
drivers/gpu/drm/i915/i915_drv.h | 3 +-
drivers/gpu/drm/i915/i915_gem.c | 5 +-
drivers/gpu/drm/i915/i915_vma.c | 15 +-
drivers/gpu/drm/i915/i915_vma.h | 3 +
12 files changed, 278 insertions(+), 54 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_domain.c b/drivers/gpu/drm/i915/gem/i915_gem_domain.c
index e2af63af67ad..d6c56272e785 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_domain.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_domain.c
@@ -255,7 +255,7 @@ int i915_gem_object_set_cache_level(struct drm_i915_gem_object *obj,
}
if (obj->userfault_count)
- __i915_gem_object_release_mmap(obj);
+ __i915_gem_object_release_mmap_gtt(obj);
/*
* As we no longer need a fence for GTT access,
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_mman.c b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
index d60973603cc1..e2df6379c7a7 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_mman.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
@@ -219,7 +219,8 @@ vm_fault_t i915_gem_fault(struct vm_fault *vmf)
{
#define MIN_CHUNK_PAGES (SZ_1M >> PAGE_SHIFT)
struct vm_area_struct *area = vmf->vma;
- struct drm_i915_gem_object *obj = to_intel_bo(area->vm_private_data);
+ struct i915_mmap_offset *priv = area->vm_private_data;
+ struct drm_i915_gem_object *obj = priv->obj;
struct drm_device *dev = obj->base.dev;
struct drm_i915_private *i915 = to_i915(dev);
struct intel_runtime_pm *rpm = &i915->runtime_pm;
@@ -312,6 +313,9 @@ vm_fault_t i915_gem_fault(struct vm_fault *vmf)
list_add(&obj->userfault_link, &i915->ggtt.userfault_list);
mutex_unlock(&i915->ggtt.vm.mutex);
+ /* Track the mmo associated with the fenced vma */
+ vma->mmo = priv;
+
if (IS_ACTIVE(CONFIG_DRM_I915_USERFAULT_AUTOSUSPEND))
intel_wakeref_auto(&i915->ggtt.userfault_wakeref,
msecs_to_jiffies_timeout(CONFIG_DRM_I915_USERFAULT_AUTOSUSPEND));
@@ -358,7 +362,7 @@ vm_fault_t i915_gem_fault(struct vm_fault *vmf)
}
}
-void __i915_gem_object_release_mmap(struct drm_i915_gem_object *obj)
+void __i915_gem_object_release_mmap_gtt(struct drm_i915_gem_object *obj)
{
struct i915_vma *vma;
@@ -366,20 +370,16 @@ void __i915_gem_object_release_mmap(struct drm_i915_gem_object *obj)
obj->userfault_count = 0;
list_del(&obj->userfault_link);
- drm_vma_node_unmap(&obj->base.vma_node,
- obj->base.dev->anon_inode->i_mapping);
- for_each_ggtt_vma(vma, obj)
+ for_each_ggtt_vma(vma, obj) {
+ if (vma->mmo)
+ drm_vma_node_unmap(&vma->mmo->vma_node,
+ obj->base.dev->anon_inode->i_mapping);
i915_vma_unset_userfault(vma);
+ }
}
/**
- * i915_gem_object_release_mmap - remove physical page mappings
- * @obj: obj in question
- *
- * Preserve the reservation of the mmapping with the DRM core code, but
- * relinquish ownership of the pages back to the system.
- *
* It is vital that we remove the page mapping if we have mapped a tiled
* object through the GTT and then lose the fence register due to
* resource pressure. Similarly if the object has been moved out of the
@@ -387,7 +387,7 @@ void __i915_gem_object_release_mmap(struct drm_i915_gem_object *obj)
* mapping will then trigger a page fault on the next user access, allowing
* fixup by i915_gem_fault().
*/
-void i915_gem_object_release_mmap(struct drm_i915_gem_object *obj)
+static void i915_gem_object_release_mmap_gtt(struct drm_i915_gem_object *obj)
{
struct drm_i915_private *i915 = to_i915(obj->base.dev);
intel_wakeref_t wakeref;
@@ -406,7 +406,7 @@ void i915_gem_object_release_mmap(struct drm_i915_gem_object *obj)
if (!obj->userfault_count)
goto out;
- __i915_gem_object_release_mmap(obj);
+ __i915_gem_object_release_mmap_gtt(obj);
/* Ensure that the CPU's PTE are revoked and there are not outstanding
* memory transactions from userspace before we return. The TLB
@@ -422,15 +422,63 @@ void i915_gem_object_release_mmap(struct drm_i915_gem_object *obj)
intel_runtime_pm_put(&i915->runtime_pm, wakeref);
}
-static int create_mmap_offset(struct drm_i915_gem_object *obj)
+void i915_gem_object_release_mmap_offset(struct drm_i915_gem_object *obj)
+{
+ struct i915_mmap_offset *mmo;
+
+ mutex_lock(&obj->mmo_lock);
+ list_for_each_entry(mmo, &obj->mmap_offsets, offset) {
+ /* vma_node_unmap for GTT mmaps handled already in
+ * __i915_gem_object_release_mmap_gtt
+ */
+ if (mmo->mmap_type != I915_MMAP_TYPE_GTT)
+ drm_vma_node_unmap(&mmo->vma_node,
+ obj->base.dev->anon_inode->i_mapping);
+ }
+ mutex_unlock(&obj->mmo_lock);
+}
+
+/**
+ * i915_gem_object_release_mmap - remove physical page mappings
+ * @obj: obj in question
+ *
+ * Preserve the reservation of the mmapping with the DRM core code, but
+ * relinquish ownership of the pages back to the system.
+ */
+void i915_gem_object_release_mmap(struct drm_i915_gem_object *obj)
+{
+ i915_gem_object_release_mmap_gtt(obj);
+ i915_gem_object_release_mmap_offset(obj);
+}
+
+static void init_mmap_offset(struct drm_i915_gem_object *obj,
+ struct i915_mmap_offset *mmo)
+{
+ mutex_lock(&obj->mmo_lock);
+ list_add(&mmo->offset, &obj->mmap_offsets);
+ mutex_unlock(&obj->mmo_lock);
+
+ mmo->obj = obj;
+ mmo->dev = obj->base.dev;
+}
+
+static int create_mmap_offset(struct drm_i915_gem_object *obj,
+ struct i915_mmap_offset *mmo)
{
struct drm_i915_private *i915 = to_i915(obj->base.dev);
struct intel_gt *gt = &i915->gt;
+ struct drm_device *dev = obj->base.dev;
int err;
- err = drm_gem_create_mmap_offset(&obj->base);
- if (likely(!err))
+ drm_vma_node_reset(&mmo->vma_node);
+ if (mmo->file)
+ drm_vma_node_allow(&mmo->vma_node, mmo->file);
+ err = drm_vma_offset_add(dev->vma_offset_manager, &mmo->vma_node,
+ obj->base.size / PAGE_SIZE);
+ if (likely(!err)) {
+ init_mmap_offset(obj, mmo);
return 0;
+ }
/* Attempt to reap some mmap space from dead objects */
err = intel_gt_retire_requests_timeout(gt, MAX_SCHEDULE_TIMEOUT);
@@ -438,34 +486,50 @@ static int create_mmap_offset(struct drm_i915_gem_object *obj)
return err;
i915_gem_drain_freed_objects(i915);
- return drm_gem_create_mmap_offset(&obj->base);
+ err = drm_vma_offset_add(dev->vma_offset_manager, &mmo->vma_node,
+ obj->base.size / PAGE_SIZE);
+ if (err)
+ return err;
+
+ init_mmap_offset(obj, mmo);
+ return 0;
}
-int
-i915_gem_mmap_gtt(struct drm_file *file,
- struct drm_device *dev,
- u32 handle,
- u64 *offset)
+static int
+__assign_gem_object_mmap_data(struct drm_file *file,
+ u32 handle,
+ enum i915_mmap_type mmap_type,
+ u64 *offset)
{
struct drm_i915_gem_object *obj;
+ struct i915_mmap_offset *mmo;
int ret;
- if (!i915_ggtt_has_aperture(&to_i915(dev)->ggtt))
- return -ENODEV;
-
obj = i915_gem_object_lookup(file, handle);
if (!obj)
return -ENOENT;
- if (i915_gem_object_never_bind_ggtt(obj)) {
+ if (mmap_type == I915_MMAP_TYPE_GTT &&
+ i915_gem_object_never_bind_ggtt(obj)) {
ret = -ENODEV;
goto out;
}
- ret = create_mmap_offset(obj);
- if (ret == 0)
- *offset = drm_vma_node_offset_addr(&obj->base.vma_node);
+ mmo = kzalloc(sizeof(*mmo), GFP_KERNEL);
+ if (!mmo) {
+ ret = -ENOMEM;
+ goto out;
+ }
+ mmo->file = file;
+ ret = create_mmap_offset(obj, mmo);
+ if (ret) {
+ kfree(mmo);
+ goto out;
+ }
+
+ mmo->mmap_type = mmap_type;
+ *offset = drm_vma_node_offset_addr(&mmo->vma_node);
out:
i915_gem_object_put(obj);
return ret;
@@ -492,7 +556,118 @@ i915_gem_mmap_gtt_ioctl(struct drm_device *dev, void *data,
{
struct drm_i915_gem_mmap_gtt *args = data;
- return i915_gem_mmap_gtt(file, dev, args->handle, &args->offset);
+ return __assign_gem_object_mmap_data(file, args->handle,
+ I915_MMAP_TYPE_GTT,
+ &args->offset);
+}
+
+void i915_mmap_offset_destroy(struct i915_mmap_offset *mmo, struct mutex *mutex)
+{
+ if (mmo->file)
+ drm_vma_node_revoke(&mmo->vma_node, mmo->file);
+ drm_vma_offset_remove(mmo->dev->vma_offset_manager, &mmo->vma_node);
+
+ mutex_lock(mutex);
+ list_del(&mmo->offset);
+ mutex_unlock(mutex);
+
+ kfree(mmo);
+}
+
+static void i915_gem_vm_open(struct vm_area_struct *vma)
+{
+ struct i915_mmap_offset *mmo = vma->vm_private_data;
+ struct drm_i915_gem_object *obj = mmo->obj;
+
+ GEM_BUG_ON(!obj);
+ i915_gem_object_get(obj);
+}
+
+static void i915_gem_vm_close(struct vm_area_struct *vma)
+{
+ struct i915_mmap_offset *mmo = vma->vm_private_data;
+ struct drm_i915_gem_object *obj = mmo->obj;
+
+ GEM_BUG_ON(!obj);
+ i915_gem_object_put(obj);
+}
+
+static const struct vm_operations_struct i915_gem_gtt_vm_ops = {
+ .fault = i915_gem_fault,
+ .open = i915_gem_vm_open,
+ .close = i915_gem_vm_close,
+};
+
+/* This overcomes the limitation in drm_gem_mmap's assignment of a
+ * drm_gem_object as the vma->vm_private_data. Since we need to
+ * be able to resolve multiple mmap offsets which could be tied
+ * to a single gem object.
+ */
+int i915_gem_mmap(struct file *filp, struct vm_area_struct *vma)
+{
+ struct drm_vma_offset_node *node;
+ struct drm_file *priv = filp->private_data;
+ struct drm_device *dev = priv->minor->dev;
+ struct i915_mmap_offset *mmo = NULL;
+ struct drm_gem_object *obj = NULL;
+
+ if (drm_dev_is_unplugged(dev))
+ return -ENODEV;
+
+ drm_vma_offset_lock_lookup(dev->vma_offset_manager);
+ node = drm_vma_offset_exact_lookup_locked(dev->vma_offset_manager,
+ vma->vm_pgoff,
+ vma_pages(vma));
+ if (likely(node)) {
+ mmo = container_of(node, struct i915_mmap_offset,
+ vma_node);
+ /*
+ * In our dependency chain, the drm_vma_offset_node
+ * depends on the validity of the mmo, which depends on
+ * the gem object. However the only reference we have
+ * at this point is the mmo (as the parent of the node).
+ * Try to check if the gem object was at least cleared.
+ */
+ if (!mmo || !mmo->obj) {
+ drm_vma_offset_unlock_lookup(dev->vma_offset_manager);
+ return -EINVAL;
+ }
+ /*
+ * Skip 0-refcnted objects as it is in the process of being
+ * destroyed and will be invalid when the vma manager lock
+ * is released.
+ */
+ obj = &mmo->obj->base;
+ if (!kref_get_unless_zero(&obj->refcount))
+ obj = NULL;
+
+ }
+ drm_vma_offset_unlock_lookup(dev->vma_offset_manager);
+
+ if (!obj)
+ return -EINVAL;
+
+ if (!drm_vma_node_is_allowed(node, priv)) {
+ drm_gem_object_put_unlocked(obj);
+ return -EACCES;
+ }
+
+ if (to_intel_bo(obj)->readonly) {
+ if (vma->vm_flags & VM_WRITE) {
+ drm_gem_object_put_unlocked(obj);
+ return -EINVAL;
+ }
+ vma->vm_flags &= ~VM_MAYWRITE;
+ }
+
+ vma->vm_flags |= VM_PFNMAP | VM_DONTEXPAND | VM_DONTDUMP;
+ vma->vm_page_prot = pgprot_writecombine(vm_get_page_prot(vma->vm_flags));
+ vma->vm_page_prot = pgprot_decrypted(vma->vm_page_prot);
+ vma->vm_private_data = mmo;
+
+ vma->vm_ops = &i915_gem_gtt_vm_ops;
+
+ return 0;
}
#if IS_ENABLED(CONFIG_DRM_I915_SELFTEST)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.c b/drivers/gpu/drm/i915/gem/i915_gem_object.c
index db103d3c8760..30c44c8fd557 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_object.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_object.c
@@ -61,6 +61,9 @@ void i915_gem_object_init(struct drm_i915_gem_object *obj,
INIT_LIST_HEAD(&obj->lut_list);
+ mutex_init(&obj->mmo_lock);
+ INIT_LIST_HEAD(&obj->mmap_offsets);
+
init_rcu_head(&obj->rcu);
obj->ops = ops;
@@ -158,6 +161,8 @@ static void __i915_gem_free_objects(struct drm_i915_private *i915,
wakeref = intel_runtime_pm_get(&i915->runtime_pm);
llist_for_each_entry_safe(obj, on, freed, freed) {
+ struct i915_mmap_offset *mmo, *mn;
+
trace_i915_gem_object_destroy(obj);
if (!list_empty(&obj->vma.list)) {
@@ -183,6 +188,14 @@ static void __i915_gem_free_objects(struct drm_i915_private *i915,
spin_unlock(&obj->vma.lock);
}
+ i915_gem_object_release_mmap(obj);
+
+ list_for_each_entry_safe(mmo, mn, &obj->mmap_offsets, offset) {
+ mmo->obj = NULL;
+ i915_mmap_offset_destroy(mmo, &obj->mmo_lock);
+ }
+
+ GEM_BUG_ON(!list_empty(&obj->mmap_offsets));
GEM_BUG_ON(atomic_read(&obj->bind_count));
GEM_BUG_ON(obj->userfault_count);
GEM_BUG_ON(!list_empty(&obj->lut_list));
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.h b/drivers/gpu/drm/i915/gem/i915_gem_object.h
index e5750d506cc9..314cba57b185 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_object.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_object.h
@@ -132,13 +132,13 @@ void i915_gem_object_unlock_fence(struct drm_i915_gem_object *obj,
static inline void
i915_gem_object_set_readonly(struct drm_i915_gem_object *obj)
{
- obj->base.vma_node.readonly = true;
+ obj->readonly = true;
}
static inline bool
i915_gem_object_is_readonly(const struct drm_i915_gem_object *obj)
{
- return obj->base.vma_node.readonly;
+ return obj->readonly;
}
static inline bool
@@ -387,8 +387,9 @@ static inline void i915_gem_object_unpin_map(struct drm_i915_gem_object *obj)
i915_gem_object_unpin_pages(obj);
}
-void __i915_gem_object_release_mmap(struct drm_i915_gem_object *obj);
+void __i915_gem_object_release_mmap_gtt(struct drm_i915_gem_object *obj);
void i915_gem_object_release_mmap(struct drm_i915_gem_object *obj);
+void i915_gem_object_release_mmap_offset(struct drm_i915_gem_object *obj);
void
i915_gem_object_flush_write_domain(struct drm_i915_gem_object *obj,
@@ -473,5 +474,6 @@ int i915_gem_object_wait(struct drm_i915_gem_object *obj,
int i915_gem_object_wait_priority(struct drm_i915_gem_object *obj,
unsigned int flags,
const struct i915_sched_attr *attr);
+void i915_mmap_offset_destroy(struct i915_mmap_offset *mmo, struct mutex *mutex);
#endif
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object_types.h b/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
index 15f8297dc34e..632d6b804cfb 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
@@ -63,6 +63,20 @@ struct drm_i915_gem_object_ops {
void (*release)(struct drm_i915_gem_object *obj);
};
+enum i915_mmap_type {
+ I915_MMAP_TYPE_GTT = 0,
+};
+
+struct i915_mmap_offset {
+ struct drm_device *dev;
+ struct drm_vma_offset_node vma_node;
+ struct drm_i915_gem_object *obj;
+ struct drm_file *file;
+ enum i915_mmap_type mmap_type;
+
+ struct list_head offset;
+};
+
struct drm_i915_gem_object {
struct drm_gem_object base;
@@ -118,6 +132,11 @@ struct drm_i915_gem_object {
unsigned int userfault_count;
struct list_head userfault_link;
+ /* Protects access to mmap offsets */
+ struct mutex mmo_lock;
+ struct list_head mmap_offsets;
+ bool readonly:1;
+
I915_SELFTEST_DECLARE(struct list_head st_link);
unsigned long flags;
diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c b/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c
index 9f1a69027a04..7761d2e0971d 100644
--- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c
+++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c
@@ -564,15 +564,20 @@ static bool assert_mmap_offset(struct drm_i915_private *i915,
int expected)
{
struct drm_i915_gem_object *obj;
+ /* Ownership transferred to parent gem object in create_mmap_offset */
+ struct i915_mmap_offset *mmo = kzalloc(sizeof(*mmo), GFP_KERNEL);
int err;
obj = i915_gem_object_create_internal(i915, size);
if (IS_ERR(obj))
return PTR_ERR(obj);
- err = create_mmap_offset(obj);
+ err = create_mmap_offset(obj, mmo);
+ if (err)
+ kfree(mmo);
i915_gem_object_put(obj);
+
return err == expected;
}
@@ -608,6 +613,8 @@ static int igt_mmap_offset_exhaustion(void *arg)
struct drm_mm *mm = &i915->drm.vma_offset_manager->vm_addr_space_mm;
struct drm_i915_gem_object *obj;
struct drm_mm_node *hole, *next;
+ /* Ownership transferred to parent gem object in create_mmap_offset */
+ struct i915_mmap_offset *mmo = kzalloc(sizeof(*mmo), GFP_KERNEL);
int loop, err;
/* Disable background reaper */
@@ -672,9 +679,10 @@ static int igt_mmap_offset_exhaustion(void *arg)
goto out;
}
- err = create_mmap_offset(obj);
+ err = create_mmap_offset(obj, mmo);
if (err) {
pr_err("Unable to insert object into reclaimed hole\n");
+ kfree(mmo);
goto err_obj;
}
diff --git a/drivers/gpu/drm/i915/gt/intel_reset.c b/drivers/gpu/drm/i915/gt/intel_reset.c
index b7007cd78c6f..777eaa483676 100644
--- a/drivers/gpu/drm/i915/gt/intel_reset.c
+++ b/drivers/gpu/drm/i915/gt/intel_reset.c
@@ -678,8 +678,13 @@ static void revoke_mmaps(struct intel_gt *gt)
continue;
GEM_BUG_ON(vma->fence != >->ggtt->fence_regs[i]);
- node = &vma->obj->base.vma_node;
+
+ if (!vma->mmo)
+ continue;
+
+ node = &vma->mmo->vma_node;
vma_offset = vma->ggtt_view.partial.offset << PAGE_SHIFT;
+
unmap_mapping_range(gt->i915->drm.anon_inode->i_mapping,
drm_vma_node_offset_addr(node) + vma_offset,
vma->size,
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 8d7049792d62..5fe071640893 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -2662,18 +2662,12 @@ const struct dev_pm_ops i915_pm_ops = {
.runtime_resume = intel_runtime_resume,
};
-static const struct vm_operations_struct i915_gem_vm_ops = {
- .fault = i915_gem_fault,
- .open = drm_gem_vm_open,
- .close = drm_gem_vm_close,
-};
-
static const struct file_operations i915_driver_fops = {
.owner = THIS_MODULE,
.open = drm_open,
.release = drm_release,
.unlocked_ioctl = drm_ioctl,
- .mmap = drm_gem_mmap,
+ .mmap = i915_gem_mmap,
.poll = drm_poll,
.read = drm_read,
.compat_ioctl = i915_compat_ioctl,
@@ -2762,7 +2756,6 @@ static struct drm_driver driver = {
.gem_close_object = i915_gem_close_object,
.gem_free_object_unlocked = i915_gem_free_object,
- .gem_vm_ops = &i915_gem_vm_ops,
.prime_handle_to_fd = drm_gem_prime_handle_to_fd,
.prime_fd_to_handle = drm_gem_prime_fd_to_handle,
@@ -2773,7 +2766,6 @@ static struct drm_driver driver = {
.get_scanout_position = i915_get_crtc_scanoutpos,
.dumb_create = i915_gem_dumb_create,
- .dumb_map_offset = i915_gem_mmap_gtt,
.ioctls = i915_ioctls,
.num_ioctls = ARRAY_SIZE(i915_ioctls),
.fops = &i915_driver_fops,
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 1779f600fcfb..968c53bfb57b 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1861,8 +1861,6 @@ i915_mutex_lock_interruptible(struct drm_device *dev)
int i915_gem_dumb_create(struct drm_file *file_priv,
struct drm_device *dev,
struct drm_mode_create_dumb *args);
-int i915_gem_mmap_gtt(struct drm_file *file_priv, struct drm_device *dev,
- u32 handle, u64 *offset);
int i915_gem_mmap_gtt_version(void);
int __must_check i915_gem_set_global_seqno(struct drm_device *dev, u32 seqno);
@@ -1886,6 +1884,7 @@ void i915_gem_driver_release(struct drm_i915_private *dev_priv);
void i915_gem_suspend(struct drm_i915_private *dev_priv);
void i915_gem_suspend_late(struct drm_i915_private *dev_priv);
void i915_gem_resume(struct drm_i915_private *dev_priv);
+int i915_gem_mmap(struct file *filp, struct vm_area_struct *vma);
vm_fault_t i915_gem_fault(struct vm_fault *vmf);
int i915_gem_open(struct drm_i915_private *i915, struct drm_file *file);
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 43c532756c7c..a1da9b6d031b 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -145,6 +145,9 @@ int i915_gem_object_unbind(struct drm_i915_gem_object *obj,
list_splice(&still_in_list, &obj->vma.list);
spin_unlock(&obj->vma.lock);
+ if (flags & I915_GEM_OBJECT_UNBIND_ACTIVE)
+ i915_gem_object_release_mmap_offset(obj);
+
return ret;
}
@@ -854,7 +857,7 @@ void i915_gem_runtime_suspend(struct drm_i915_private *i915)
list_for_each_entry_safe(obj, on,
&i915->ggtt.userfault_list, userfault_link)
- __i915_gem_object_release_mmap(obj);
+ __i915_gem_object_release_mmap_gtt(obj);
/*
* The fence will be lost when the device powers down. If any were
diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c
index e5512f26e20a..8c1aab9b9627 100644
--- a/drivers/gpu/drm/i915/i915_vma.c
+++ b/drivers/gpu/drm/i915/i915_vma.c
@@ -1054,7 +1054,7 @@ static void __i915_vma_iounmap(struct i915_vma *vma)
void i915_vma_revoke_mmap(struct i915_vma *vma)
{
- struct drm_vma_offset_node *node = &vma->obj->base.vma_node;
+ struct drm_vma_offset_node *node;
u64 vma_offset;
lockdep_assert_held(&vma->vm->mutex);
@@ -1066,10 +1066,15 @@ void i915_vma_revoke_mmap(struct i915_vma *vma)
GEM_BUG_ON(!vma->obj->userfault_count);
vma_offset = vma->ggtt_view.partial.offset << PAGE_SHIFT;
- unmap_mapping_range(vma->vm->i915->drm.anon_inode->i_mapping,
- drm_vma_node_offset_addr(node) + vma_offset,
- vma->size,
- 1);
+
+ if (vma->mmo) {
+ node = &vma->mmo->vma_node;
+
+ unmap_mapping_range(vma->vm->i915->drm.anon_inode->i_mapping,
+ drm_vma_node_offset_addr(node) + vma_offset,
+ vma->size,
+ 1);
+ }
i915_vma_unset_userfault(vma);
if (!--vma->obj->userfault_count)
diff --git a/drivers/gpu/drm/i915/i915_vma.h b/drivers/gpu/drm/i915/i915_vma.h
index 465932813bc5..f09f4f513c41 100644
--- a/drivers/gpu/drm/i915/i915_vma.h
+++ b/drivers/gpu/drm/i915/i915_vma.h
@@ -63,6 +63,9 @@ struct i915_vma {
u64 display_alignment;
struct i915_page_sizes page_sizes;
+ /* mmap-offset associated with fencing for this vma */
+ struct i915_mmap_offset *mmo;
+
u32 fence_size;
u32 fence_alignment;
--
2.23.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 28+ messages in thread
end of thread, other threads:[~2019-11-19 11:37 UTC | newest]
Thread overview: 28+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-15 11:45 [PATCH 1/4] drm/i915: Allow i915 to manage the vma offset nodes instead of drm core Abdiel Janulgue
2019-11-15 11:45 ` [Intel-gfx] " Abdiel Janulgue
2019-11-15 11:45 ` [PATCH 2/4] drm/i915: Introduce DRM_I915_GEM_MMAP_OFFSET Abdiel Janulgue
2019-11-15 11:45 ` [Intel-gfx] " Abdiel Janulgue
2019-11-15 13:51 ` Chris Wilson
2019-11-15 13:51 ` [Intel-gfx] " Chris Wilson
2019-11-15 14:31 ` Chris Wilson
2019-11-15 14:31 ` [Intel-gfx] " Chris Wilson
2019-11-15 11:45 ` [PATCH 3/4] drm/i915: cpu-map based dumb buffers Abdiel Janulgue
2019-11-15 11:45 ` [Intel-gfx] " Abdiel Janulgue
2019-11-15 13:54 ` Chris Wilson
2019-11-15 13:54 ` [Intel-gfx] " Chris Wilson
2019-11-15 14:26 ` Chris Wilson
2019-11-15 14:26 ` [Intel-gfx] " Chris Wilson
2019-11-15 11:45 ` [PATCH 4/4] drm/i915: Add cpu fault handler for mmap_offset Abdiel Janulgue
2019-11-15 11:45 ` [Intel-gfx] " Abdiel Janulgue
2019-11-15 13:58 ` Chris Wilson
2019-11-15 13:58 ` [Intel-gfx] " Chris Wilson
2019-11-15 13:56 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/4] drm/i915: Allow i915 to manage the vma offset nodes instead of drm core Patchwork
2019-11-15 13:56 ` [Intel-gfx] " Patchwork
2019-11-15 13:57 ` ✗ Fi.CI.SPARSE: " Patchwork
2019-11-15 13:57 ` [Intel-gfx] " Patchwork
2019-11-15 14:17 ` [PATCH 1/4] " Chris Wilson
2019-11-15 14:17 ` [Intel-gfx] " Chris Wilson
2019-11-15 14:23 ` ✗ Fi.CI.BAT: failure for series starting with [1/4] " Patchwork
2019-11-15 14:23 ` [Intel-gfx] " Patchwork
-- strict thread matches above, loose matches on Subject: below --
2019-11-19 11:37 [PATCH 1/4] " Abdiel Janulgue
2019-11-14 19:09 Abdiel Janulgue
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.