* [PATCH 1/5] drm/i915: Allow i915 to manage the vma offset nodes instead of drm core
@ 2019-08-26 12:20 Abdiel Janulgue
2019-08-26 12:20 ` [PATCH 2/5] drm/i915: Introduce DRM_I915_GEM_MMAP_OFFSET Abdiel Janulgue
` (12 more replies)
0 siblings, 13 replies; 28+ messages in thread
From: Abdiel Janulgue @ 2019-08-26 12:20 UTC (permalink / raw)
To: intel-gfx
Have i915 replace the core drm_gem_mmap implementation to overcome its
limitation in having only a single mmap offset node per gem object.
The change allows us to have multiple mmap offsets per object. This
enables a mmapping instance to use unique fault-handlers per user vma.
This enables us 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.
Signed-off-by: Abdiel Janulgue <abdiel.janulgue@linux.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_mman.c | 193 ++++++++++++++++--
drivers/gpu/drm/i915/gem/i915_gem_object.c | 19 ++
drivers/gpu/drm/i915/gem/i915_gem_object.h | 7 +-
.../gpu/drm/i915/gem/i915_gem_object_types.h | 18 ++
.../drm/i915/gem/selftests/i915_gem_mman.c | 12 +-
drivers/gpu/drm/i915/gt/intel_reset.c | 11 +-
drivers/gpu/drm/i915/i915_drv.c | 9 +-
drivers/gpu/drm/i915/i915_drv.h | 1 +
drivers/gpu/drm/i915/i915_vma.c | 20 +-
9 files changed, 254 insertions(+), 36 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_mman.c b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
index 595539a09e38..fb7e39f115d7 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_mman.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
@@ -218,7 +218,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;
@@ -372,13 +373,20 @@ vm_fault_t i915_gem_fault(struct vm_fault *vmf)
void __i915_gem_object_release_mmap(struct drm_i915_gem_object *obj)
{
struct i915_vma *vma;
+ struct i915_mmap_offset *mmo;
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);
+
+ mutex_lock(&obj->mmo_lock);
+ list_for_each_entry(mmo, &obj->mmap_offsets, offset) {
+ 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);
for_each_ggtt_vma(vma, obj)
i915_vma_unset_userfault(vma);
@@ -433,14 +441,33 @@ 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)
+static void init_mmap_offset(struct drm_i915_gem_object *obj,
+ struct i915_mmap_offset *mmo)
+{
+ mutex_lock(&obj->mmo_lock);
+ kref_init(&mmo->ref);
+ list_add(&mmo->offset, &obj->mmap_offsets);
+ mutex_unlock(&obj->mmo_lock);
+
+ mmo->obj = obj;
+}
+
+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 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 */
do {
@@ -451,32 +478,48 @@ static int create_mmap_offset(struct drm_i915_gem_object *obj)
break;
i915_gem_drain_freed_objects(i915);
- err = drm_gem_create_mmap_offset(&obj->base);
- if (!err)
+ err = drm_vma_offset_add(dev->vma_offset_manager, &mmo->vma_node,
+ obj->base.size / PAGE_SIZE);
+ if (!err) {
+ init_mmap_offset(obj, mmo);
break;
+ }
} while (flush_delayed_work(&i915->gem.retire_work));
return err;
}
-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;
obj = i915_gem_object_lookup(file, handle);
if (!obj)
return -ENOENT;
- 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 err;
+ }
+
+ mmo->file = file;
+ ret = create_mmap_offset(obj, mmo);
+ if (ret) {
+ kfree(mmo);
+ goto err;
+ }
+ mmo->mmap_type = mmap_type;
+ *offset = drm_vma_node_offset_addr(&mmo->vma_node);
+err:
i915_gem_object_put(obj);
return ret;
}
@@ -500,9 +543,123 @@ int
i915_gem_mmap_gtt_ioctl(struct drm_device *dev, void *data,
struct drm_file *file)
{
- struct drm_i915_gem_mmap_gtt *args = data;
+ struct drm_i915_gem_mmap_offset *args = data;
+
+ return __assign_gem_object_mmap_data(file, args->handle,
+ I915_MMAP_TYPE_GTT,
+ &args->offset);
+}
+
+void i915_mmap_offset_object_release(struct kref *ref)
+{
+ struct i915_mmap_offset *mmo = container_of(ref,
+ struct i915_mmap_offset,
+ ref);
+ struct drm_i915_gem_object *obj = mmo->obj;
+ struct drm_device *dev = obj->base.dev;
+
+ lockdep_assert_held(&obj->mmo_lock);
+
+ if (mmo->file)
+ drm_vma_node_revoke(&mmo->vma_node, mmo->file);
+ drm_vma_offset_remove(dev->vma_offset_manager, &mmo->vma_node);
+ list_del(&mmo->offset);
- return i915_gem_mmap_gtt(file, dev, args->handle, &args->offset);
+ kfree(mmo);
+}
+
+static void i915_gem_vm_open(struct vm_area_struct *vma)
+{
+ struct i915_mmap_offset *priv = vma->vm_private_data;
+ struct drm_i915_gem_object *obj = priv->obj;
+
+ i915_gem_object_get(obj);
+ kref_get(&priv->ref);
+}
+
+static void i915_gem_vm_close(struct vm_area_struct *vma)
+{
+ struct i915_mmap_offset *priv = vma->vm_private_data;
+ struct drm_i915_gem_object *obj = priv->obj;
+
+ mutex_lock(&obj->mmo_lock);
+ kref_put(&priv->ref, i915_mmap_offset_object_release);
+ mutex_unlock(&obj->mmo_lock);
+
+ 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);
+ /*
+ * 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;
+
+ /*
+ * Take a ref for our mmap_offset object. The reference is cleaned
+ * up when the vma is closed.
+ */
+ kref_get(&mmo->ref);
+
+ 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 d7855dc5a5c5..eb3d7f3528f3 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_object.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_object.c
@@ -58,6 +58,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;
@@ -94,6 +97,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);
@@ -108,6 +112,13 @@ void i915_gem_close_object(struct drm_gem_object *gem, struct drm_file *file)
}
i915_gem_object_unlock(obj);
+ mutex_lock(&obj->mmo_lock);
+ list_for_each_entry_safe(mmo, on, &obj->mmap_offsets, offset) {
+ if (mmo->file == file)
+ kref_put(&mmo->ref, i915_mmap_offset_object_release);
+ }
+ mutex_unlock(&obj->mmo_lock);
+
list_for_each_entry_safe(lut, ln, &close, obj_link) {
struct i915_gem_context *ctx = lut->ctx;
struct i915_vma *vma;
@@ -156,6 +167,7 @@ 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_vma *vma, *vn;
+ struct i915_mmap_offset *mmo, *on;
trace_i915_gem_object_destroy(obj);
@@ -169,6 +181,13 @@ static void __i915_gem_free_objects(struct drm_i915_private *i915,
GEM_BUG_ON(!list_empty(&obj->vma.list));
GEM_BUG_ON(!RB_EMPTY_ROOT(&obj->vma.tree));
+ i915_gem_object_release_mmap(obj);
+
+ mutex_lock(&obj->mmo_lock);
+ list_for_each_entry_safe(mmo, on, &obj->mmap_offsets, offset)
+ kref_put(&mmo->ref, i915_mmap_offset_object_release);
+ mutex_unlock(&obj->mmo_lock);
+
mutex_unlock(&i915->drm.struct_mutex);
GEM_BUG_ON(atomic_read(&obj->bind_count));
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.h b/drivers/gpu/drm/i915/gem/i915_gem_object.h
index 5efb9936e05b..d667ed8bb0a6 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_object.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_object.h
@@ -125,13 +125,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
@@ -423,6 +423,9 @@ 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_object_release(struct kref *ref);
+
#define I915_PRIORITY_DISPLAY I915_USER_PRIORITY(I915_PRIORITY_MAX)
#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 ede0eb4218a8..d74ddb479318 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
@@ -60,6 +60,19 @@ 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_vma_offset_node vma_node;
+ struct drm_i915_gem_object *obj;
+ struct drm_file *file;
+ enum i915_mmap_type mmap_type;
+ struct kref ref;
+ struct list_head offset;
+};
+
struct drm_i915_gem_object {
struct drm_gem_object base;
@@ -115,6 +128,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);
/*
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 1d27babff0ce..a7513421b1db 100644
--- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c
+++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c
@@ -371,15 +371,20 @@ static bool assert_mmap_offset(struct drm_i915_private *i915,
int expected)
{
struct drm_i915_gem_object *obj;
+ /* refcounted 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;
}
@@ -422,6 +427,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 resv, *hole;
+ /* refcounted in create_mmap_offset */
+ struct i915_mmap_offset *mmo = kzalloc(sizeof(*mmo), GFP_KERNEL);
u64 hole_start, hole_end;
int loop, err;
@@ -465,9 +472,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 b9d84d52e986..7b82ef888b96 100644
--- a/drivers/gpu/drm/i915/gt/intel_reset.c
+++ b/drivers/gpu/drm/i915/gt/intel_reset.c
@@ -628,6 +628,7 @@ static void revoke_mmaps(struct intel_gt *gt)
for (i = 0; i < gt->ggtt->num_fences; i++) {
struct drm_vma_offset_node *node;
+ struct i915_mmap_offset *mmo;
struct i915_vma *vma;
u64 vma_offset;
@@ -641,10 +642,18 @@ static void revoke_mmaps(struct intel_gt *gt)
GEM_BUG_ON(vma->fence != >->ggtt->fence_regs[i]);
node = &vma->obj->base.vma_node;
vma_offset = vma->ggtt_view.partial.offset << PAGE_SHIFT;
- unmap_mapping_range(gt->i915->drm.anon_inode->i_mapping,
+
+ list_for_each_entry(mmo, &vma->obj->mmap_offsets, offset) {
+ node = &mmo->vma_node;
+ if (!drm_mm_node_allocated(&node->vm_node) ||
+ mmo->mmap_type != I915_MMAP_TYPE_GTT)
+ continue;
+
+ unmap_mapping_range(gt->i915->drm.anon_inode->i_mapping,
drm_vma_node_offset_addr(node) + vma_offset,
vma->size,
1);
+ }
}
}
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 1c4576a4a5e9..bdd597200984 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -2722,18 +2722,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,
@@ -2822,7 +2816,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,
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index b42651a387d9..bf2927e88e14 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2327,6 +2327,7 @@ int i915_gem_wait_for_idle(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_vma.c b/drivers/gpu/drm/i915/i915_vma.c
index e0e677b2a3a9..677323644319 100644
--- a/drivers/gpu/drm/i915/i915_vma.c
+++ b/drivers/gpu/drm/i915/i915_vma.c
@@ -861,7 +861,8 @@ 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;
+ struct i915_mmap_offset *mmo;
u64 vma_offset;
lockdep_assert_held(&vma->vm->mutex);
@@ -873,10 +874,19 @@ 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);
+
+ list_for_each_entry(mmo, &vma->obj->mmap_offsets, offset) {
+ node = &mmo->vma_node;
+ /* Only gtt-mmaps for this vma should be unmapped */
+ if (!drm_mm_node_allocated(&node->vm_node) ||
+ mmo->mmap_type != I915_MMAP_TYPE_GTT)
+ continue;
+
+ 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)
--
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/5] drm/i915: Introduce DRM_I915_GEM_MMAP_OFFSET
2019-08-26 12:20 [PATCH 1/5] drm/i915: Allow i915 to manage the vma offset nodes instead of drm core Abdiel Janulgue
@ 2019-08-26 12:20 ` Abdiel Janulgue
2019-08-26 12:21 ` [PATCH 3/5] drm/i915: Add cpu fault handler for mmap_offset Abdiel Janulgue
` (11 subsequent siblings)
12 siblings, 0 replies; 28+ messages in thread
From: Abdiel Janulgue @ 2019-08-26 12:20 UTC (permalink / raw)
To: intel-gfx; +Cc: Matthew Auld
This is really just an alias of mmap_gtt. Add a new CPU mmap
implementation that allows multiple 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.
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 | 34 ++++++++++++++++++-
.../gpu/drm/i915/gem/i915_gem_object_types.h | 3 ++
drivers/gpu/drm/i915/i915_drv.c | 2 +-
include/uapi/drm/i915_drm.h | 28 +++++++++++++++
4 files changed, 65 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_mman.c b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
index fb7e39f115d7..87690aa64814 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_mman.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
@@ -143,6 +143,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
@@ -170,7 +173,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
@@ -524,6 +527,27 @@ __assign_gem_object_mmap_data(struct drm_file *file,
return ret;
}
+static int gem_mmap_offset(struct drm_device *dev, void *data,
+ struct drm_file *file)
+{
+ struct drm_i915_gem_mmap_offset *args = data;
+ enum i915_mmap_type type;
+
+ if ((args->flags & (I915_MMAP_OFFSET_WC | I915_MMAP_OFFSET_WB)) &&
+ !boot_cpu_has(X86_FEATURE_PAT))
+ return -ENODEV;
+
+ if (args->flags & I915_MMAP_OFFSET_WC)
+ type = I915_MMAP_TYPE_OFFSET_WC;
+ else if (args->flags & I915_MMAP_OFFSET_WB)
+ type = I915_MMAP_TYPE_OFFSET_WB;
+ else if (args->flags & I915_MMAP_OFFSET_UC)
+ type = I915_MMAP_TYPE_OFFSET_UC;
+
+ return __assign_gem_object_mmap_data(file, args->handle, type,
+ &args->offset);
+}
+
/**
* i915_gem_mmap_gtt_ioctl - prepare an object for GTT mmap'ing
* @dev: DRM device
@@ -544,6 +568,14 @@ i915_gem_mmap_gtt_ioctl(struct drm_device *dev, void *data,
struct drm_file *file)
{
struct drm_i915_gem_mmap_offset *args = data;
+ struct drm_i915_private *i915 = to_i915(dev);
+
+ if (args->flags & I915_MMAP_OFFSET_FLAGS)
+ return gem_mmap_offset(dev, data, file);
+
+ if (!HAS_MAPPABLE_APERTURE(i915))
+ /* No aperture, cannot mmap via legacy GTT */
+ return -ENODEV;
return __assign_gem_object_mmap_data(file, args->handle,
I915_MMAP_TYPE_GTT,
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 d74ddb479318..cf7712aea2d9 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
@@ -62,6 +62,9 @@ struct drm_i915_gem_object_ops {
enum i915_mmap_type {
I915_MMAP_TYPE_GTT = 0,
+ I915_MMAP_TYPE_OFFSET_WC,
+ I915_MMAP_TYPE_OFFSET_WB,
+ I915_MMAP_TYPE_OFFSET_UC,
};
struct i915_mmap_offset {
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index bdd597200984..68ed4499fa52 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -2774,7 +2774,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_gtt_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/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index 469dc512cca3..75fc9277e9ab 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -359,6 +359,7 @@ typedef struct _drm_i915_sarea {
#define DRM_I915_QUERY 0x39
#define DRM_I915_GEM_VM_CREATE 0x3a
#define DRM_I915_GEM_VM_DESTROY 0x3b
+#define DRM_I915_GEM_MMAP_OFFSET DRM_I915_GEM_MMAP_GTT
/* Must be kept compact -- no holes */
#define DRM_IOCTL_I915_INIT DRM_IOW( DRM_COMMAND_BASE + DRM_I915_INIT, drm_i915_init_t)
@@ -421,6 +422,7 @@ typedef struct _drm_i915_sarea {
#define DRM_IOCTL_I915_QUERY DRM_IOWR(DRM_COMMAND_BASE + DRM_I915_QUERY, struct drm_i915_query)
#define DRM_IOCTL_I915_GEM_VM_CREATE DRM_IOWR(DRM_COMMAND_BASE + DRM_I915_GEM_VM_CREATE, struct drm_i915_gem_vm_control)
#define DRM_IOCTL_I915_GEM_VM_DESTROY DRM_IOW (DRM_COMMAND_BASE + DRM_I915_GEM_VM_DESTROY, struct drm_i915_gem_vm_control)
+#define DRM_IOCTL_I915_GEM_MMAP_OFFSET DRM_IOWR(DRM_COMMAND_BASE + DRM_I915_GEM_MMAP_OFFSET, struct drm_i915_gem_mmap_offset)
/* Allow drivers to submit batchbuffers directly to hardware, relying
* on the security mechanisms provided by hardware.
@@ -611,6 +613,7 @@ typedef struct drm_i915_irq_wait {
* See I915_EXEC_FENCE_OUT and I915_EXEC_FENCE_SUBMIT.
*/
#define I915_PARAM_HAS_EXEC_SUBMIT_FENCE 53
+
/* Must be kept compact -- no holes and well documented */
typedef struct drm_i915_getparam {
@@ -786,6 +789,31 @@ 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 _WC/_WB flags
+ * should be passed here.
+ */
+ __u64 flags;
+#define I915_MMAP_OFFSET_WC (1 << 0)
+#define I915_MMAP_OFFSET_WB (1 << 1)
+#define I915_MMAP_OFFSET_UC (1 << 2)
+#define I915_MMAP_OFFSET_FLAGS \
+ (I915_MMAP_OFFSET_WC | I915_MMAP_OFFSET_WB | I915_MMAP_OFFSET_UC)
+};
+
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/5] drm/i915: Add cpu fault handler for mmap_offset
2019-08-26 12:20 [PATCH 1/5] drm/i915: Allow i915 to manage the vma offset nodes instead of drm core Abdiel Janulgue
2019-08-26 12:20 ` [PATCH 2/5] drm/i915: Introduce DRM_I915_GEM_MMAP_OFFSET Abdiel Janulgue
@ 2019-08-26 12:21 ` Abdiel Janulgue
2019-08-26 12:44 ` Chris Wilson
2019-08-26 12:46 ` Chris Wilson
2019-08-26 12:21 ` [PATCH 4/5] drm/i915: define HAS_MAPPABLE_APERTURE Abdiel Janulgue
` (10 subsequent siblings)
12 siblings, 2 replies; 28+ messages in thread
From: Abdiel Janulgue @ 2019-08-26 12:21 UTC (permalink / raw)
To: intel-gfx; +Cc: Matthew Auld
Fault handler to handle missing pages for shmem-backed objects. Handle also
logic needed to refault pages depending on fault handler usage.
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_domain.c | 2 +-
drivers/gpu/drm/i915/gem/i915_gem_mman.c | 189 ++++++++++++++++-----
drivers/gpu/drm/i915/gem/i915_gem_object.h | 2 +-
drivers/gpu/drm/i915/i915_gem.c | 2 +-
4 files changed, 149 insertions(+), 46 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_domain.c b/drivers/gpu/drm/i915/gem/i915_gem_domain.c
index 9c58e8fac1d9..517e73c3aa31 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_domain.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_domain.c
@@ -254,7 +254,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 87690aa64814..d0a653a9a4cc 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"
@@ -199,6 +200,45 @@ compute_partial_view(const struct drm_i915_gem_object *obj,
return view;
}
+static vm_fault_t i915_error_to_vmf_fault(int err, struct intel_gt *gt)
+{
+ switch (err) {
+ case -EIO:
+ /*
+ * We eat errors when the gpu is terminally wedged to avoid
+ * userspace unduly crashing (gl has no provisions for mmaps to
+ * fail). But any other -EIO isn't ours (e.g. swap in failure)
+ * and so needs to be reported.
+ */
+ if (!intel_gt_is_wedged(gt))
+ return VM_FAULT_SIGBUS;
+ /* else, fall through */
+ case -EAGAIN:
+ /*
+ * EAGAIN means the gpu is hung and we'll wait for the error
+ * handler to reset everything when re-faulting in
+ * i915_mutex_lock_interruptible.
+ */
+ case 0:
+ case -ERESTARTSYS:
+ case -EINTR:
+ case -EBUSY:
+ /*
+ * EBUSY is ok: this just means that another thread
+ * already did the job.
+ */
+ return VM_FAULT_NOPAGE;
+ case -ENOMEM:
+ return VM_FAULT_OOM;
+ case -ENOSPC:
+ case -EFAULT:
+ return VM_FAULT_SIGBUS;
+ default:
+ WARN_ONCE(err, "unhandled error in %s: %i\n", __func__, err);
+ return VM_FAULT_SIGBUS;
+ }
+}
+
/**
* i915_gem_fault - fault a page into the GTT
* @vmf: fault info
@@ -336,44 +376,48 @@ 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) {
- case -EIO:
- /*
- * We eat errors when the gpu is terminally wedged to avoid
- * userspace unduly crashing (gl has no provisions for mmaps to
- * fail). But any other -EIO isn't ours (e.g. swap in failure)
- * and so needs to be reported.
- */
- if (!intel_gt_is_wedged(ggtt->vm.gt))
- return VM_FAULT_SIGBUS;
- /* else, fall through */
- case -EAGAIN:
- /*
- * EAGAIN means the gpu is hung and we'll wait for the error
- * handler to reset everything when re-faulting in
- * i915_mutex_lock_interruptible.
- */
- case 0:
- case -ERESTARTSYS:
- case -EINTR:
- case -EBUSY:
- /*
- * EBUSY is ok: this just means that another thread
- * already did the job.
- */
- return VM_FAULT_NOPAGE;
- case -ENOMEM:
- return VM_FAULT_OOM;
- case -ENOSPC:
- case -EFAULT:
- return VM_FAULT_SIGBUS;
- default:
- WARN_ONCE(ret, "unhandled error in %s: %i\n", __func__, ret);
+ return i915_error_to_vmf_fault(ret, ggtt->vm.gt);
+}
+
+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;
+ struct drm_device *dev = obj->base.dev;
+ struct drm_i915_private *dev_priv = to_i915(dev);
+ vm_fault_t vmf_ret;
+ unsigned long size = area->vm_end - area->vm_start;
+ bool write = area->vm_flags & VM_WRITE;
+ int i, ret;
+
+ /* 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, &dev_priv->gt);
+
+ 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_ERROR) {
+ vmf_ret = i915_error_to_vmf_fault(vm_fault_to_errno(vmf_ret, 0),
+ &dev_priv->gt);
+ break;
+ }
}
+
+ i915_gem_object_unpin_pages(obj);
+
+ return vmf_ret;
}
-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;
struct i915_mmap_offset *mmo;
@@ -396,12 +440,6 @@ void __i915_gem_object_release_mmap(struct drm_i915_gem_object *obj)
}
/**
- * 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
@@ -409,7 +447,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;
@@ -428,7 +466,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
@@ -444,6 +482,35 @@ void i915_gem_object_release_mmap(struct drm_i915_gem_object *obj)
intel_runtime_pm_put(&i915->runtime_pm, wakeref);
}
+static 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)
{
@@ -627,6 +694,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_OFFSET_WC:
+ vma->vm_page_prot =
+ pgprot_writecombine(vm_get_page_prot(vma->vm_flags));
+ break;
+ case I915_MMAP_TYPE_OFFSET_WB:
+ vma->vm_page_prot = vm_get_page_prot(vma->vm_flags);
+ break;
+ case I915_MMAP_TYPE_OFFSET_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
@@ -683,7 +777,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_OFFSET_WC:
+ case I915_MMAP_TYPE_OFFSET_WB:
+ case I915_MMAP_TYPE_OFFSET_UC:
+ set_vmdata_mmap_offset(mmo, vma);
+ break;
+ case I915_MMAP_TYPE_GTT:
+ vma->vm_ops = &i915_gem_gtt_vm_ops;
+ break;
+ }
/*
* Take a ref for our mmap_offset object. The reference is cleaned
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.h b/drivers/gpu/drm/i915/gem/i915_gem_object.h
index d667ed8bb0a6..012e5d1d99cc 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_object.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_object.h
@@ -338,7 +338,7 @@ 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
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 95e7c52cf8ed..363b35ba559d 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -857,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
--
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/5] drm/i915: define HAS_MAPPABLE_APERTURE
2019-08-26 12:20 [PATCH 1/5] drm/i915: Allow i915 to manage the vma offset nodes instead of drm core Abdiel Janulgue
2019-08-26 12:20 ` [PATCH 2/5] drm/i915: Introduce DRM_I915_GEM_MMAP_OFFSET Abdiel Janulgue
2019-08-26 12:21 ` [PATCH 3/5] drm/i915: Add cpu fault handler for mmap_offset Abdiel Janulgue
@ 2019-08-26 12:21 ` Abdiel Janulgue
2019-08-26 12:21 ` [PATCH 5/5] drm/i915: cpu-map based dumb buffers Abdiel Janulgue
` (9 subsequent siblings)
12 siblings, 0 replies; 28+ messages in thread
From: Abdiel Janulgue @ 2019-08-26 12:21 UTC (permalink / raw)
To: intel-gfx; +Cc: Matthew Auld
From: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
The following patches in the series will use it to avoid certain
operations when aperture is not available in HW.
Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Matthew Auld <matthew.auld@intel.com>
---
drivers/gpu/drm/i915/i915_drv.h | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index bf2927e88e14..4e1d2e0fcc86 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2110,6 +2110,8 @@ IS_SUBPLATFORM(const struct drm_i915_private *i915,
#define OVERLAY_NEEDS_PHYSICAL(dev_priv) \
(INTEL_INFO(dev_priv)->display.overlay_needs_physical)
+#define HAS_MAPPABLE_APERTURE(dev_priv) (dev_priv->ggtt.mappable_end > 0)
+
/* Early gen2 have a totally busted CS tlb and require pinned batches. */
#define HAS_BROKEN_CS_TLB(dev_priv) (IS_I830(dev_priv) || IS_I845G(dev_priv))
--
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 5/5] drm/i915: cpu-map based dumb buffers
2019-08-26 12:20 [PATCH 1/5] drm/i915: Allow i915 to manage the vma offset nodes instead of drm core Abdiel Janulgue
` (2 preceding siblings ...)
2019-08-26 12:21 ` [PATCH 4/5] drm/i915: define HAS_MAPPABLE_APERTURE Abdiel Janulgue
@ 2019-08-26 12:21 ` Abdiel Janulgue
2019-08-26 12:48 ` Chris Wilson
2019-08-26 12:42 ` [PATCH 1/5] drm/i915: Allow i915 to manage the vma offset nodes instead of drm core Chris Wilson
` (8 subsequent siblings)
12 siblings, 1 reply; 28+ messages in thread
From: Abdiel Janulgue @ 2019-08-26 12:21 UTC (permalink / raw)
To: intel-gfx; +Cc: Matthew Auld
If there is no aperture we can't use map_gtt to map dumb buffers, so we
need a cpu-map based path to do it. We prefer map_gtt on platforms that
do have aperture.
Signed-off-by: Abdiel Janulgue <abdiel.janulgue@linux.intel.com>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
Cc: Matthew Auld <matthew.auld@intel.com>
---
drivers/gpu/drm/i915/gem/i915_gem_mman.c | 13 +++++++++++++
drivers/gpu/drm/i915/i915_drv.c | 2 +-
drivers/gpu/drm/i915/i915_drv.h | 2 +-
3 files changed, 15 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_mman.c b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
index d0a653a9a4cc..ff35b0517e59 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_mman.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
@@ -615,6 +615,19 @@ static int gem_mmap_offset(struct drm_device *dev, void *data,
&args->offset);
}
+int
+i915_gem_mmap_dumb(struct drm_file *file,
+ struct drm_device *dev,
+ u32 handle,
+ u64 *offset)
+{
+ struct drm_i915_private *i915 = dev->dev_private;
+ enum i915_mmap_type mmap_type = HAS_MAPPABLE_APERTURE(i915) ?
+ I915_MMAP_TYPE_GTT : I915_MMAP_TYPE_OFFSET_WC;
+
+ return __assign_gem_object_mmap_data(file, handle, mmap_type, offset);
+}
+
/**
* i915_gem_mmap_gtt_ioctl - prepare an object for GTT mmap'ing
* @dev: DRM device
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 68ed4499fa52..40ccadc9ef29 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -2826,7 +2826,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_gtt,
+ .dumb_map_offset = i915_gem_mmap_dumb,
.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 4e1d2e0fcc86..d3514184efef 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2300,7 +2300,7 @@ 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,
+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);
--
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 1/5] drm/i915: Allow i915 to manage the vma offset nodes instead of drm core
2019-08-26 12:20 [PATCH 1/5] drm/i915: Allow i915 to manage the vma offset nodes instead of drm core Abdiel Janulgue
` (3 preceding siblings ...)
2019-08-26 12:21 ` [PATCH 5/5] drm/i915: cpu-map based dumb buffers Abdiel Janulgue
@ 2019-08-26 12:42 ` Chris Wilson
2019-08-26 12:51 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/5] " Patchwork
` (7 subsequent siblings)
12 siblings, 0 replies; 28+ messages in thread
From: Chris Wilson @ 2019-08-26 12:42 UTC (permalink / raw)
To: Abdiel Janulgue, intel-gfx
Quoting Abdiel Janulgue (2019-08-26 13:20:58)
> -static int create_mmap_offset(struct drm_i915_gem_object *obj)
> +static void init_mmap_offset(struct drm_i915_gem_object *obj,
> + struct i915_mmap_offset *mmo)
> +{
> + mutex_lock(&obj->mmo_lock);
This lock should only be guarding obj->mmap_offsets. You don't need to
take it around every kref, just be careful to remove the link on close.
> + kref_init(&mmo->ref);
> + list_add(&mmo->offset, &obj->mmap_offsets);
> + mutex_unlock(&obj->mmo_lock);
> +
> + mmo->obj = obj;
> +}
> +/* 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);
> + /*
> + * 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;
Hmm, references are still weird. This doesn't seem like it protects
against
Thread A Thread B
mmap(fd, offset_of_A); gem_close(fd, A);
Time for a gem_mmap_gtt/close-race.
-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/5] drm/i915: Add cpu fault handler for mmap_offset
2019-08-26 12:21 ` [PATCH 3/5] drm/i915: Add cpu fault handler for mmap_offset Abdiel Janulgue
@ 2019-08-26 12:44 ` Chris Wilson
2019-08-26 12:46 ` Chris Wilson
1 sibling, 0 replies; 28+ messages in thread
From: Chris Wilson @ 2019-08-26 12:44 UTC (permalink / raw)
To: Abdiel Janulgue, intel-gfx; +Cc: Matthew Auld
Quoting Abdiel Janulgue (2019-08-26 13:21:00)
> Fault handler to handle missing pages for shmem-backed objects. Handle also
> logic needed to refault pages depending on fault handler usage.
>
> 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_domain.c | 2 +-
> drivers/gpu/drm/i915/gem/i915_gem_mman.c | 189 ++++++++++++++++-----
> drivers/gpu/drm/i915/gem/i915_gem_object.h | 2 +-
> drivers/gpu/drm/i915/i915_gem.c | 2 +-
> 4 files changed, 149 insertions(+), 46 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_domain.c b/drivers/gpu/drm/i915/gem/i915_gem_domain.c
> index 9c58e8fac1d9..517e73c3aa31 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_domain.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_domain.c
> @@ -254,7 +254,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 87690aa64814..d0a653a9a4cc 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"
> @@ -199,6 +200,45 @@ compute_partial_view(const struct drm_i915_gem_object *obj,
> return view;
> }
>
> +static vm_fault_t i915_error_to_vmf_fault(int err, struct intel_gt *gt)
> +{
> + switch (err) {
> + case -EIO:
> + /*
> + * We eat errors when the gpu is terminally wedged to avoid
> + * userspace unduly crashing (gl has no provisions for mmaps to
> + * fail). But any other -EIO isn't ours (e.g. swap in failure)
> + * and so needs to be reported.
> + */
> + if (!intel_gt_is_wedged(gt))
> + return VM_FAULT_SIGBUS;
> + /* else, fall through */
> + case -EAGAIN:
> + /*
> + * EAGAIN means the gpu is hung and we'll wait for the error
> + * handler to reset everything when re-faulting in
> + * i915_mutex_lock_interruptible.
> + */
These errors don't exist anymore and should not be cargo-culted onto
paths that could not possibly generate them.
-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/5] drm/i915: Add cpu fault handler for mmap_offset
2019-08-26 12:21 ` [PATCH 3/5] drm/i915: Add cpu fault handler for mmap_offset Abdiel Janulgue
2019-08-26 12:44 ` Chris Wilson
@ 2019-08-26 12:46 ` Chris Wilson
1 sibling, 0 replies; 28+ messages in thread
From: Chris Wilson @ 2019-08-26 12:46 UTC (permalink / raw)
To: Abdiel Janulgue, intel-gfx; +Cc: Matthew Auld
Quoting Abdiel Janulgue (2019-08-26 13:21:00)
> + 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_ERROR) {
> + vmf_ret = i915_error_to_vmf_fault(vm_fault_to_errno(vmf_ret, 0),
> + &dev_priv->gt);
Sigh. You take a mm/, run it through the internal error convertor that
*only* applies to i915 errors, and then return the original fault code.
-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 5/5] drm/i915: cpu-map based dumb buffers
2019-08-26 12:21 ` [PATCH 5/5] drm/i915: cpu-map based dumb buffers Abdiel Janulgue
@ 2019-08-26 12:48 ` Chris Wilson
2019-08-27 8:31 ` Abdiel Janulgue
0 siblings, 1 reply; 28+ messages in thread
From: Chris Wilson @ 2019-08-26 12:48 UTC (permalink / raw)
To: Abdiel Janulgue, intel-gfx; +Cc: Matthew Auld
Quoting Abdiel Janulgue (2019-08-26 13:21:02)
> +int
> +i915_gem_mmap_dumb(struct drm_file *file,
> + struct drm_device *dev,
> + u32 handle,
> + u64 *offset)
> +{
> + struct drm_i915_private *i915 = dev->dev_private;
> + enum i915_mmap_type mmap_type = HAS_MAPPABLE_APERTURE(i915) ?
> + I915_MMAP_TYPE_GTT : I915_MMAP_TYPE_OFFSET_WC;
Is there any reason for using GTT if we support WC? The buffer is meant
to be linear.
-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/5] drm/i915: Allow i915 to manage the vma offset nodes instead of drm core
2019-08-26 12:20 [PATCH 1/5] drm/i915: Allow i915 to manage the vma offset nodes instead of drm core Abdiel Janulgue
` (4 preceding siblings ...)
2019-08-26 12:42 ` [PATCH 1/5] drm/i915: Allow i915 to manage the vma offset nodes instead of drm core Chris Wilson
@ 2019-08-26 12:51 ` Patchwork
2019-08-26 12:52 ` ✗ Fi.CI.SPARSE: " Patchwork
` (6 subsequent siblings)
12 siblings, 0 replies; 28+ messages in thread
From: Patchwork @ 2019-08-26 12:51 UTC (permalink / raw)
To: Abdiel Janulgue; +Cc: intel-gfx
== Series Details ==
Series: series starting with [1/5] drm/i915: Allow i915 to manage the vma offset nodes instead of drm core
URL : https://patchwork.freedesktop.org/series/65793/
State : warning
== Summary ==
$ dim checkpatch origin/drm-tip
f5bf965f51a8 drm/i915: Allow i915 to manage the vma offset nodes instead of drm core
-:425: CHECK:LINE_SPACING: Please don't use multiple blank lines
#425: FILE: drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c:387:
+
-:475: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#475: FILE: drivers/gpu/drm/i915/gt/intel_reset.c:653:
+ unmap_mapping_range(gt->i915->drm.anon_inode->i_mapping,
drm_vma_node_offset_addr(node) + vma_offset,
total: 0 errors, 0 warnings, 2 checks, 484 lines checked
b1e0a0f8d0d9 drm/i915: Introduce DRM_I915_GEM_MMAP_OFFSET
-:127: WARNING:LONG_LINE: line over 100 characters
#127: FILE: include/uapi/drm/i915_drm.h:425:
+#define DRM_IOCTL_I915_GEM_MMAP_OFFSET DRM_IOWR(DRM_COMMAND_BASE + DRM_I915_GEM_MMAP_OFFSET, struct drm_i915_gem_mmap_offset)
total: 0 errors, 1 warnings, 0 checks, 127 lines checked
2824d8871cc6 drm/i915: Add cpu fault handler for mmap_offset
a15b2877b235 drm/i915: define HAS_MAPPABLE_APERTURE
-:20: CHECK:MACRO_ARG_PRECEDENCE: Macro argument 'dev_priv' may be better as '(dev_priv)' to avoid precedence issues
#20: FILE: drivers/gpu/drm/i915/i915_drv.h:2113:
+#define HAS_MAPPABLE_APERTURE(dev_priv) (dev_priv->ggtt.mappable_end > 0)
total: 0 errors, 0 warnings, 1 checks, 8 lines checked
db6a41bc2840 drm/i915: cpu-map based dumb buffers
-:25: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#25: FILE: drivers/gpu/drm/i915/gem/i915_gem_mman.c:620:
+i915_gem_mmap_dumb(struct drm_file *file,
+ struct drm_device *dev,
-:62: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#62: FILE: drivers/gpu/drm/i915/i915_drv.h:2304:
+int i915_gem_mmap_dumb(struct drm_file *file_priv, struct drm_device *dev,
u32 handle, u64 *offset);
total: 0 errors, 0 warnings, 2 checks, 35 lines checked
_______________________________________________
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/5] drm/i915: Allow i915 to manage the vma offset nodes instead of drm core
2019-08-26 12:20 [PATCH 1/5] drm/i915: Allow i915 to manage the vma offset nodes instead of drm core Abdiel Janulgue
` (5 preceding siblings ...)
2019-08-26 12:51 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/5] " Patchwork
@ 2019-08-26 12:52 ` Patchwork
2019-08-26 12:53 ` [PATCH 1/5] " Chris Wilson
` (5 subsequent siblings)
12 siblings, 0 replies; 28+ messages in thread
From: Patchwork @ 2019-08-26 12:52 UTC (permalink / raw)
To: Abdiel Janulgue; +Cc: intel-gfx
== Series Details ==
Series: series starting with [1/5] drm/i915: Allow i915 to manage the vma offset nodes instead of drm core
URL : https://patchwork.freedesktop.org/series/65793/
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 1/5] drm/i915: Allow i915 to manage the vma offset nodes instead of drm core
2019-08-26 12:20 [PATCH 1/5] drm/i915: Allow i915 to manage the vma offset nodes instead of drm core Abdiel Janulgue
` (6 preceding siblings ...)
2019-08-26 12:52 ` ✗ Fi.CI.SPARSE: " Patchwork
@ 2019-08-26 12:53 ` Chris Wilson
2019-08-26 13:36 ` ✓ Fi.CI.BAT: success for series starting with [1/5] " Patchwork
` (4 subsequent siblings)
12 siblings, 0 replies; 28+ messages in thread
From: Chris Wilson @ 2019-08-26 12:53 UTC (permalink / raw)
To: Abdiel Janulgue, intel-gfx
Quoting Abdiel Janulgue (2019-08-26 13:20:58)
> @@ -641,10 +642,18 @@ static void revoke_mmaps(struct intel_gt *gt)
> GEM_BUG_ON(vma->fence != >->ggtt->fence_regs[i]);
> node = &vma->obj->base.vma_node;
> vma_offset = vma->ggtt_view.partial.offset << PAGE_SHIFT;
> - unmap_mapping_range(gt->i915->drm.anon_inode->i_mapping,
> +
> + list_for_each_entry(mmo, &vma->obj->mmap_offsets, offset) {
> + node = &mmo->vma_node;
> + if (!drm_mm_node_allocated(&node->vm_node) ||
> + mmo->mmap_type != I915_MMAP_TYPE_GTT)
> + continue;
That list needs locking as is not protected by the reset srcu (and you
are not allowed your own locking in here, unless you have a jolly good
reason and can prove it will never block a reset).
One thing to observe is is that you only ever need the mmo associated
with a fence for revocation upon reset, and you could use a local list
for tracking the active mmo.
-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: success for series starting with [1/5] drm/i915: Allow i915 to manage the vma offset nodes instead of drm core
2019-08-26 12:20 [PATCH 1/5] drm/i915: Allow i915 to manage the vma offset nodes instead of drm core Abdiel Janulgue
` (7 preceding siblings ...)
2019-08-26 12:53 ` [PATCH 1/5] " Chris Wilson
@ 2019-08-26 13:36 ` Patchwork
2019-08-26 15:00 ` ✗ Fi.CI.IGT: failure " Patchwork
` (3 subsequent siblings)
12 siblings, 0 replies; 28+ messages in thread
From: Patchwork @ 2019-08-26 13:36 UTC (permalink / raw)
To: Abdiel Janulgue; +Cc: intel-gfx
== Series Details ==
Series: series starting with [1/5] drm/i915: Allow i915 to manage the vma offset nodes instead of drm core
URL : https://patchwork.freedesktop.org/series/65793/
State : success
== Summary ==
CI Bug Log - changes from CI_DRM_6784 -> Patchwork_14188
====================================================
Summary
-------
**SUCCESS**
No regressions found.
External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14188/
Known issues
------------
Here are the changes found in Patchwork_14188 that come from known issues:
### IGT changes ###
#### Issues hit ####
* igt@kms_flip@basic-flip-vs-dpms:
- fi-hsw-4770r: [PASS][1] -> [DMESG-WARN][2] ([fdo#105602])
[1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6784/fi-hsw-4770r/igt@kms_flip@basic-flip-vs-dpms.html
[2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14188/fi-hsw-4770r/igt@kms_flip@basic-flip-vs-dpms.html
* igt@kms_frontbuffer_tracking@basic:
- fi-icl-u3: [PASS][3] -> [FAIL][4] ([fdo#103167])
[3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6784/fi-icl-u3/igt@kms_frontbuffer_tracking@basic.html
[4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14188/fi-icl-u3/igt@kms_frontbuffer_tracking@basic.html
- fi-icl-u2: [PASS][5] -> [FAIL][6] ([fdo#103167])
[5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6784/fi-icl-u2/igt@kms_frontbuffer_tracking@basic.html
[6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14188/fi-icl-u2/igt@kms_frontbuffer_tracking@basic.html
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
[fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
[fdo#105602]: https://bugs.freedesktop.org/show_bug.cgi?id=105602
[fdo#111045]: https://bugs.freedesktop.org/show_bug.cgi?id=111045
Participating hosts (54 -> 43)
------------------------------
Missing (11): fi-kbl-soraka fi-ilk-m540 fi-bsw-n3050 fi-byt-j1900 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-icl-y fi-blb-e6850 fi-byt-clapper fi-bdw-samus
Build changes
-------------
* CI: CI-20190529 -> None
* Linux: CI_DRM_6784 -> Patchwork_14188
CI-20190529: 20190529
CI_DRM_6784: 38e1a03497a6e7d1dd271f3f0cd140127181053d @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_5149: 6756ede680ee12745393360d7cc87cc0eb733ff6 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_14188: db6a41bc284019cc6835513c15c96d4e7c4a7c8f @ git://anongit.freedesktop.org/gfx-ci/linux
== Linux commits ==
db6a41bc2840 drm/i915: cpu-map based dumb buffers
a15b2877b235 drm/i915: define HAS_MAPPABLE_APERTURE
2824d8871cc6 drm/i915: Add cpu fault handler for mmap_offset
b1e0a0f8d0d9 drm/i915: Introduce DRM_I915_GEM_MMAP_OFFSET
f5bf965f51a8 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_14188/
_______________________________________________
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.IGT: failure for series starting with [1/5] drm/i915: Allow i915 to manage the vma offset nodes instead of drm core
2019-08-26 12:20 [PATCH 1/5] drm/i915: Allow i915 to manage the vma offset nodes instead of drm core Abdiel Janulgue
` (8 preceding siblings ...)
2019-08-26 13:36 ` ✓ Fi.CI.BAT: success for series starting with [1/5] " Patchwork
@ 2019-08-26 15:00 ` Patchwork
2019-09-04 10:33 ` [PATCH 1/5] " Daniel Vetter
` (2 subsequent siblings)
12 siblings, 0 replies; 28+ messages in thread
From: Patchwork @ 2019-08-26 15:00 UTC (permalink / raw)
To: Abdiel Janulgue; +Cc: intel-gfx
== Series Details ==
Series: series starting with [1/5] drm/i915: Allow i915 to manage the vma offset nodes instead of drm core
URL : https://patchwork.freedesktop.org/series/65793/
State : failure
== Summary ==
CI Bug Log - changes from CI_DRM_6784_full -> Patchwork_14188_full
====================================================
Summary
-------
**FAILURE**
Serious unknown changes coming with Patchwork_14188_full absolutely need to be
verified manually.
If you think the reported changes have nothing to do with the changes
introduced in Patchwork_14188_full, please notify your bug team to allow them
to document this new failure mode, which will reduce false positives in CI.
Possible new issues
-------------------
Here are the unknown changes that may have been introduced in Patchwork_14188_full:
### IGT changes ###
#### Possible regressions ####
* igt@gem_persistent_relocs@forked-interruptible-faulting-reloc-thrashing:
- shard-hsw: [PASS][1] -> [TIMEOUT][2]
[1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6784/shard-hsw2/igt@gem_persistent_relocs@forked-interruptible-faulting-reloc-thrashing.html
[2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14188/shard-hsw2/igt@gem_persistent_relocs@forked-interruptible-faulting-reloc-thrashing.html
Known issues
------------
Here are the changes found in Patchwork_14188_full that come from known issues:
### IGT changes ###
#### Issues hit ####
* igt@gem_exec_schedule@reorder-wide-bsd:
- shard-iclb: [PASS][3] -> [SKIP][4] ([fdo#111325]) +7 similar issues
[3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6784/shard-iclb8/igt@gem_exec_schedule@reorder-wide-bsd.html
[4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14188/shard-iclb2/igt@gem_exec_schedule@reorder-wide-bsd.html
* igt@gem_tiled_swapping@non-threaded:
- shard-apl: [PASS][5] -> [DMESG-WARN][6] ([fdo#108686])
[5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6784/shard-apl7/igt@gem_tiled_swapping@non-threaded.html
[6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14188/shard-apl7/igt@gem_tiled_swapping@non-threaded.html
- shard-glk: [PASS][7] -> [DMESG-WARN][8] ([fdo#108686])
[7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6784/shard-glk2/igt@gem_tiled_swapping@non-threaded.html
[8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14188/shard-glk2/igt@gem_tiled_swapping@non-threaded.html
* igt@gem_workarounds@suspend-resume-context:
- shard-apl: [PASS][9] -> [DMESG-WARN][10] ([fdo#108566]) +4 similar issues
[9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6784/shard-apl6/igt@gem_workarounds@suspend-resume-context.html
[10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14188/shard-apl3/igt@gem_workarounds@suspend-resume-context.html
* igt@kms_flip@flip-vs-expired-vblank-interruptible:
- shard-glk: [PASS][11] -> [FAIL][12] ([fdo#105363]) +1 similar issue
[11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6784/shard-glk7/igt@kms_flip@flip-vs-expired-vblank-interruptible.html
[12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14188/shard-glk6/igt@kms_flip@flip-vs-expired-vblank-interruptible.html
* igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-spr-indfb-draw-render:
- shard-iclb: [PASS][13] -> [FAIL][14] ([fdo#103167]) +2 similar issues
[13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6784/shard-iclb6/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-spr-indfb-draw-render.html
[14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14188/shard-iclb2/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-spr-indfb-draw-render.html
* igt@kms_plane_alpha_blend@pipe-c-coverage-7efc:
- shard-skl: [PASS][15] -> [FAIL][16] ([fdo#108145] / [fdo#110403])
[15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6784/shard-skl7/igt@kms_plane_alpha_blend@pipe-c-coverage-7efc.html
[16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14188/shard-skl4/igt@kms_plane_alpha_blend@pipe-c-coverage-7efc.html
* igt@kms_psr@psr2_cursor_plane_onoff:
- shard-iclb: [PASS][17] -> [SKIP][18] ([fdo#109441]) +1 similar issue
[17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6784/shard-iclb2/igt@kms_psr@psr2_cursor_plane_onoff.html
[18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14188/shard-iclb7/igt@kms_psr@psr2_cursor_plane_onoff.html
* igt@prime_busy@hang-bsd2:
- shard-iclb: [PASS][19] -> [SKIP][20] ([fdo#109276]) +23 similar issues
[19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6784/shard-iclb2/igt@prime_busy@hang-bsd2.html
[20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14188/shard-iclb8/igt@prime_busy@hang-bsd2.html
#### Possible fixes ####
* igt@gem_ctx_shared@exec-single-timeline-bsd:
- shard-iclb: [SKIP][21] ([fdo#110841]) -> [PASS][22]
[21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6784/shard-iclb2/igt@gem_ctx_shared@exec-single-timeline-bsd.html
[22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14188/shard-iclb8/igt@gem_ctx_shared@exec-single-timeline-bsd.html
* igt@gem_exec_schedule@wide-bsd:
- shard-iclb: [SKIP][23] ([fdo#111325]) -> [PASS][24] +4 similar issues
[23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6784/shard-iclb4/igt@gem_exec_schedule@wide-bsd.html
[24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14188/shard-iclb3/igt@gem_exec_schedule@wide-bsd.html
* igt@gem_exec_suspend@basic-s3:
- shard-kbl: [DMESG-WARN][25] ([fdo#108566]) -> [PASS][26]
[25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6784/shard-kbl3/igt@gem_exec_suspend@basic-s3.html
[26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14188/shard-kbl6/igt@gem_exec_suspend@basic-s3.html
* igt@gem_softpin@noreloc-s3:
- shard-skl: [INCOMPLETE][27] ([fdo#104108]) -> [PASS][28]
[27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6784/shard-skl6/igt@gem_softpin@noreloc-s3.html
[28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14188/shard-skl6/igt@gem_softpin@noreloc-s3.html
* igt@i915_suspend@sysfs-reader:
- shard-apl: [DMESG-WARN][29] ([fdo#108566]) -> [PASS][30] +3 similar issues
[29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6784/shard-apl4/igt@i915_suspend@sysfs-reader.html
[30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14188/shard-apl5/igt@i915_suspend@sysfs-reader.html
* igt@kms_cursor_legacy@2x-long-cursor-vs-flip-atomic:
- shard-hsw: [FAIL][31] ([fdo#105767]) -> [PASS][32]
[31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6784/shard-hsw1/igt@kms_cursor_legacy@2x-long-cursor-vs-flip-atomic.html
[32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14188/shard-hsw7/igt@kms_cursor_legacy@2x-long-cursor-vs-flip-atomic.html
* igt@kms_frontbuffer_tracking@fbc-1p-primscrn-indfb-pgflip-blt:
- shard-iclb: [FAIL][33] ([fdo#103167]) -> [PASS][34] +3 similar issues
[33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6784/shard-iclb2/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-indfb-pgflip-blt.html
[34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14188/shard-iclb4/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-indfb-pgflip-blt.html
* igt@kms_psr@psr2_sprite_plane_move:
- shard-iclb: [SKIP][35] ([fdo#109441]) -> [PASS][36] +2 similar issues
[35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6784/shard-iclb3/igt@kms_psr@psr2_sprite_plane_move.html
[36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14188/shard-iclb2/igt@kms_psr@psr2_sprite_plane_move.html
* igt@kms_setmode@basic:
- shard-apl: [FAIL][37] ([fdo#99912]) -> [PASS][38]
[37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6784/shard-apl5/igt@kms_setmode@basic.html
[38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14188/shard-apl6/igt@kms_setmode@basic.html
* igt@kms_vblank@pipe-b-query-busy:
- shard-apl: [INCOMPLETE][39] ([fdo#103927]) -> [PASS][40] +3 similar issues
[39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6784/shard-apl6/igt@kms_vblank@pipe-b-query-busy.html
[40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14188/shard-apl7/igt@kms_vblank@pipe-b-query-busy.html
* igt@perf@polling:
- shard-skl: [FAIL][41] ([fdo#110728]) -> [PASS][42]
[41]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6784/shard-skl3/igt@perf@polling.html
[42]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14188/shard-skl10/igt@perf@polling.html
* igt@prime_vgem@fence-wait-bsd2:
- shard-iclb: [SKIP][43] ([fdo#109276]) -> [PASS][44] +15 similar issues
[43]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6784/shard-iclb6/igt@prime_vgem@fence-wait-bsd2.html
[44]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14188/shard-iclb2/igt@prime_vgem@fence-wait-bsd2.html
#### Warnings ####
* igt@gem_ctx_isolation@vcs1-nonpriv:
- shard-iclb: [FAIL][45] ([fdo#111329]) -> [SKIP][46] ([fdo#109276])
[45]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6784/shard-iclb2/igt@gem_ctx_isolation@vcs1-nonpriv.html
[46]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14188/shard-iclb8/igt@gem_ctx_isolation@vcs1-nonpriv.html
* igt@gem_mocs_settings@mocs-reset-bsd2:
- shard-iclb: [FAIL][47] ([fdo#111330]) -> [SKIP][48] ([fdo#109276])
[47]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6784/shard-iclb4/igt@gem_mocs_settings@mocs-reset-bsd2.html
[48]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14188/shard-iclb6/igt@gem_mocs_settings@mocs-reset-bsd2.html
* igt@gem_mocs_settings@mocs-settings-bsd2:
- shard-iclb: [SKIP][49] ([fdo#109276]) -> [FAIL][50] ([fdo#111330]) +1 similar issue
[49]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6784/shard-iclb8/igt@gem_mocs_settings@mocs-settings-bsd2.html
[50]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14188/shard-iclb2/igt@gem_mocs_settings@mocs-settings-bsd2.html
* igt@kms_frontbuffer_tracking@fbc-badstride:
- shard-skl: [FAIL][51] ([fdo#108040]) -> [FAIL][52] ([fdo#103167])
[51]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6784/shard-skl5/igt@kms_frontbuffer_tracking@fbc-badstride.html
[52]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14188/shard-skl6/igt@kms_frontbuffer_tracking@fbc-badstride.html
* igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-spr-indfb-fullscreen:
- shard-skl: [FAIL][53] ([fdo#103167]) -> [FAIL][54] ([fdo#108040])
[53]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6784/shard-skl5/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-spr-indfb-fullscreen.html
[54]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14188/shard-skl6/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-spr-indfb-fullscreen.html
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
[fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
[fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
[fdo#104108]: https://bugs.freedesktop.org/show_bug.cgi?id=104108
[fdo#105363]: https://bugs.freedesktop.org/show_bug.cgi?id=105363
[fdo#105767]: https://bugs.freedesktop.org/show_bug.cgi?id=105767
[fdo#108040]: https://bugs.freedesktop.org/show_bug.cgi?id=108040
[fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
[fdo#108566]: https://bugs.freedesktop.org/show_bug.cgi?id=108566
[fdo#108686]: https://bugs.freedesktop.org/show_bug.cgi?id=108686
[fdo#109276]: https://bugs.freedesktop.org/show_bug.cgi?id=109276
[fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
[fdo#110403]: https://bugs.freedesktop.org/show_bug.cgi?id=110403
[fdo#110728]: https://bugs.freedesktop.org/show_bug.cgi?id=110728
[fdo#110841]: https://bugs.freedesktop.org/show_bug.cgi?id=110841
[fdo#111325]: https://bugs.freedesktop.org/show_bug.cgi?id=111325
[fdo#111329]: https://bugs.freedesktop.org/show_bug.cgi?id=111329
[fdo#111330]: https://bugs.freedesktop.org/show_bug.cgi?id=111330
[fdo#99912]: https://bugs.freedesktop.org/show_bug.cgi?id=99912
Participating hosts (9 -> 9)
------------------------------
No changes in participating hosts
Build changes
-------------
* CI: CI-20190529 -> None
* Linux: CI_DRM_6784 -> Patchwork_14188
CI-20190529: 20190529
CI_DRM_6784: 38e1a03497a6e7d1dd271f3f0cd140127181053d @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_5149: 6756ede680ee12745393360d7cc87cc0eb733ff6 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_14188: db6a41bc284019cc6835513c15c96d4e7c4a7c8f @ git://anongit.freedesktop.org/gfx-ci/linux
piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14188/
_______________________________________________
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 5/5] drm/i915: cpu-map based dumb buffers
2019-08-26 12:48 ` Chris Wilson
@ 2019-08-27 8:31 ` Abdiel Janulgue
0 siblings, 0 replies; 28+ messages in thread
From: Abdiel Janulgue @ 2019-08-27 8:31 UTC (permalink / raw)
To: Chris Wilson, intel-gfx; +Cc: Matthew Auld
On 26/08/2019 15.48, Chris Wilson wrote:
> Quoting Abdiel Janulgue (2019-08-26 13:21:02)
>> +int
>> +i915_gem_mmap_dumb(struct drm_file *file,
>> + struct drm_device *dev,
>> + u32 handle,
>> + u64 *offset)
>> +{
>> + struct drm_i915_private *i915 = dev->dev_private;
>> + enum i915_mmap_type mmap_type = HAS_MAPPABLE_APERTURE(i915) ?
>> + I915_MMAP_TYPE_GTT : I915_MMAP_TYPE_OFFSET_WC;
>
> Is there any reason for using GTT if we support WC? The buffer is meant
> to be linear.
> -Chris
>
Actually there seems no reason at all. Yeah dumb BO mmaps should
probably be done via mmap_offset CPU WC mmaps.
-Abdiel
_______________________________________________
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/5] drm/i915: Allow i915 to manage the vma offset nodes instead of drm core
2019-08-26 12:20 [PATCH 1/5] drm/i915: Allow i915 to manage the vma offset nodes instead of drm core Abdiel Janulgue
` (9 preceding siblings ...)
2019-08-26 15:00 ` ✗ Fi.CI.IGT: failure " Patchwork
@ 2019-09-04 10:33 ` Daniel Vetter
[not found] ` <D5FCAD9A-8C9C-48ED-8ACD-40D24A572AF0@intel.com>
2019-10-07 8:48 ` [PATCH v2 " Abdiel Janulgue
2019-10-07 9:00 ` ✗ Fi.CI.BUILD: failure for series starting with [v2,5/5] drm/i915: Add cpu fault handler for mmap_offset (rev5) Patchwork
12 siblings, 1 reply; 28+ messages in thread
From: Daniel Vetter @ 2019-09-04 10:33 UTC (permalink / raw)
To: Abdiel Janulgue, Dave Airlie; +Cc: intel-gfx
On Mon, Aug 26, 2019 at 2:21 PM Abdiel Janulgue
<abdiel.janulgue@linux.intel.com> wrote:
>
> Have i915 replace the core drm_gem_mmap implementation to overcome its
> limitation in having only a single mmap offset node per gem object.
> The change allows us to have multiple mmap offsets per object. This
> enables a mmapping instance to use unique fault-handlers per user vma.
>
> This enables us 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.
>
> Signed-off-by: Abdiel Janulgue <abdiel.janulgue@linux.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_mman.c | 193 ++++++++++++++++--
> drivers/gpu/drm/i915/gem/i915_gem_object.c | 19 ++
> drivers/gpu/drm/i915/gem/i915_gem_object.h | 7 +-
> .../gpu/drm/i915/gem/i915_gem_object_types.h | 18 ++
> .../drm/i915/gem/selftests/i915_gem_mman.c | 12 +-
> drivers/gpu/drm/i915/gt/intel_reset.c | 11 +-
> drivers/gpu/drm/i915/i915_drv.c | 9 +-
> drivers/gpu/drm/i915/i915_drv.h | 1 +
> drivers/gpu/drm/i915/i915_vma.c | 20 +-
> 9 files changed, 254 insertions(+), 36 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_mman.c b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
> index 595539a09e38..fb7e39f115d7 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_mman.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
> @@ -218,7 +218,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;
> @@ -372,13 +373,20 @@ vm_fault_t i915_gem_fault(struct vm_fault *vmf)
> void __i915_gem_object_release_mmap(struct drm_i915_gem_object *obj)
> {
> struct i915_vma *vma;
> + struct i915_mmap_offset *mmo;
>
> 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);
> +
> + mutex_lock(&obj->mmo_lock);
> + list_for_each_entry(mmo, &obj->mmap_offsets, offset) {
> + 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);
>
> for_each_ggtt_vma(vma, obj)
> i915_vma_unset_userfault(vma);
> @@ -433,14 +441,33 @@ 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)
> +static void init_mmap_offset(struct drm_i915_gem_object *obj,
> + struct i915_mmap_offset *mmo)
> +{
> + mutex_lock(&obj->mmo_lock);
> + kref_init(&mmo->ref);
> + list_add(&mmo->offset, &obj->mmap_offsets);
> + mutex_unlock(&obj->mmo_lock);
> +
> + mmo->obj = obj;
> +}
> +
> +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 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 */
> do {
> @@ -451,32 +478,48 @@ static int create_mmap_offset(struct drm_i915_gem_object *obj)
> break;
>
> i915_gem_drain_freed_objects(i915);
> - err = drm_gem_create_mmap_offset(&obj->base);
> - if (!err)
> + err = drm_vma_offset_add(dev->vma_offset_manager, &mmo->vma_node,
> + obj->base.size / PAGE_SIZE);
> + if (!err) {
> + init_mmap_offset(obj, mmo);
> break;
> + }
>
> } while (flush_delayed_work(&i915->gem.retire_work));
>
> return err;
> }
>
> -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;
>
> obj = i915_gem_object_lookup(file, handle);
> if (!obj)
> return -ENOENT;
>
> - ret = create_mmap_offset(obj);
> - if (ret == 0)
> - *offset = drm_vma_node_offset_addr(&obj->base.vma_node);
> + mmo = kzalloc(sizeof(*mmo), GFP_KERNEL);
I got thrown off a bunch of times here reading the code, but I think I
got this right now.
Why exactly do we want multiple vma offsets? Yes this makes it a
drop-in replacement for the old cpu mmap ioctl, which was a bit
dubious design. But if we go all new here, I really wonder about why
this is necessary. No other discrete driver needs this, they all fix
the mmap mode for the lifetime of an object, because flushing stuff is
as expensive as just reallocating (or at least close enough).
I think us going once again our separate route here needs a lot more
justification than just "we've accidentally ended up with uapi like
this 10 years ago".
-Daniel
> + if (!mmo) {
> + ret = -ENOMEM;
> + goto err;
> + }
> +
> + mmo->file = file;
> + ret = create_mmap_offset(obj, mmo);
> + if (ret) {
> + kfree(mmo);
> + goto err;
> + }
>
> + mmo->mmap_type = mmap_type;
> + *offset = drm_vma_node_offset_addr(&mmo->vma_node);
> +err:
> i915_gem_object_put(obj);
> return ret;
> }
> @@ -500,9 +543,123 @@ int
> i915_gem_mmap_gtt_ioctl(struct drm_device *dev, void *data,
> struct drm_file *file)
> {
> - struct drm_i915_gem_mmap_gtt *args = data;
> + struct drm_i915_gem_mmap_offset *args = data;
> +
> + return __assign_gem_object_mmap_data(file, args->handle,
> + I915_MMAP_TYPE_GTT,
> + &args->offset);
> +}
> +
> +void i915_mmap_offset_object_release(struct kref *ref)
> +{
> + struct i915_mmap_offset *mmo = container_of(ref,
> + struct i915_mmap_offset,
> + ref);
> + struct drm_i915_gem_object *obj = mmo->obj;
> + struct drm_device *dev = obj->base.dev;
> +
> + lockdep_assert_held(&obj->mmo_lock);
> +
> + if (mmo->file)
> + drm_vma_node_revoke(&mmo->vma_node, mmo->file);
> + drm_vma_offset_remove(dev->vma_offset_manager, &mmo->vma_node);
> + list_del(&mmo->offset);
>
> - return i915_gem_mmap_gtt(file, dev, args->handle, &args->offset);
> + kfree(mmo);
> +}
> +
> +static void i915_gem_vm_open(struct vm_area_struct *vma)
> +{
> + struct i915_mmap_offset *priv = vma->vm_private_data;
> + struct drm_i915_gem_object *obj = priv->obj;
> +
> + i915_gem_object_get(obj);
> + kref_get(&priv->ref);
> +}
> +
> +static void i915_gem_vm_close(struct vm_area_struct *vma)
> +{
> + struct i915_mmap_offset *priv = vma->vm_private_data;
> + struct drm_i915_gem_object *obj = priv->obj;
> +
> + mutex_lock(&obj->mmo_lock);
> + kref_put(&priv->ref, i915_mmap_offset_object_release);
> + mutex_unlock(&obj->mmo_lock);
> +
> + 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);
> + /*
> + * 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;
> +
> + /*
> + * Take a ref for our mmap_offset object. The reference is cleaned
> + * up when the vma is closed.
> + */
> + kref_get(&mmo->ref);
> +
> + 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 d7855dc5a5c5..eb3d7f3528f3 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_object.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_object.c
> @@ -58,6 +58,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;
> @@ -94,6 +97,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);
> @@ -108,6 +112,13 @@ void i915_gem_close_object(struct drm_gem_object *gem, struct drm_file *file)
> }
> i915_gem_object_unlock(obj);
>
> + mutex_lock(&obj->mmo_lock);
> + list_for_each_entry_safe(mmo, on, &obj->mmap_offsets, offset) {
> + if (mmo->file == file)
> + kref_put(&mmo->ref, i915_mmap_offset_object_release);
> + }
> + mutex_unlock(&obj->mmo_lock);
> +
> list_for_each_entry_safe(lut, ln, &close, obj_link) {
> struct i915_gem_context *ctx = lut->ctx;
> struct i915_vma *vma;
> @@ -156,6 +167,7 @@ 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_vma *vma, *vn;
> + struct i915_mmap_offset *mmo, *on;
>
> trace_i915_gem_object_destroy(obj);
>
> @@ -169,6 +181,13 @@ static void __i915_gem_free_objects(struct drm_i915_private *i915,
> GEM_BUG_ON(!list_empty(&obj->vma.list));
> GEM_BUG_ON(!RB_EMPTY_ROOT(&obj->vma.tree));
>
> + i915_gem_object_release_mmap(obj);
> +
> + mutex_lock(&obj->mmo_lock);
> + list_for_each_entry_safe(mmo, on, &obj->mmap_offsets, offset)
> + kref_put(&mmo->ref, i915_mmap_offset_object_release);
> + mutex_unlock(&obj->mmo_lock);
> +
> mutex_unlock(&i915->drm.struct_mutex);
>
> GEM_BUG_ON(atomic_read(&obj->bind_count));
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.h b/drivers/gpu/drm/i915/gem/i915_gem_object.h
> index 5efb9936e05b..d667ed8bb0a6 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_object.h
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_object.h
> @@ -125,13 +125,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
> @@ -423,6 +423,9 @@ 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_object_release(struct kref *ref);
> +
> #define I915_PRIORITY_DISPLAY I915_USER_PRIORITY(I915_PRIORITY_MAX)
>
> #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 ede0eb4218a8..d74ddb479318 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
> @@ -60,6 +60,19 @@ 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_vma_offset_node vma_node;
> + struct drm_i915_gem_object *obj;
> + struct drm_file *file;
> + enum i915_mmap_type mmap_type;
> + struct kref ref;
> + struct list_head offset;
> +};
> +
> struct drm_i915_gem_object {
> struct drm_gem_object base;
>
> @@ -115,6 +128,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);
>
> /*
> 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 1d27babff0ce..a7513421b1db 100644
> --- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c
> +++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c
> @@ -371,15 +371,20 @@ static bool assert_mmap_offset(struct drm_i915_private *i915,
> int expected)
> {
> struct drm_i915_gem_object *obj;
> + /* refcounted 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;
> }
>
> @@ -422,6 +427,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 resv, *hole;
> + /* refcounted in create_mmap_offset */
> + struct i915_mmap_offset *mmo = kzalloc(sizeof(*mmo), GFP_KERNEL);
> u64 hole_start, hole_end;
> int loop, err;
>
> @@ -465,9 +472,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 b9d84d52e986..7b82ef888b96 100644
> --- a/drivers/gpu/drm/i915/gt/intel_reset.c
> +++ b/drivers/gpu/drm/i915/gt/intel_reset.c
> @@ -628,6 +628,7 @@ static void revoke_mmaps(struct intel_gt *gt)
>
> for (i = 0; i < gt->ggtt->num_fences; i++) {
> struct drm_vma_offset_node *node;
> + struct i915_mmap_offset *mmo;
> struct i915_vma *vma;
> u64 vma_offset;
>
> @@ -641,10 +642,18 @@ static void revoke_mmaps(struct intel_gt *gt)
> GEM_BUG_ON(vma->fence != >->ggtt->fence_regs[i]);
> node = &vma->obj->base.vma_node;
> vma_offset = vma->ggtt_view.partial.offset << PAGE_SHIFT;
> - unmap_mapping_range(gt->i915->drm.anon_inode->i_mapping,
> +
> + list_for_each_entry(mmo, &vma->obj->mmap_offsets, offset) {
> + node = &mmo->vma_node;
> + if (!drm_mm_node_allocated(&node->vm_node) ||
> + mmo->mmap_type != I915_MMAP_TYPE_GTT)
> + continue;
> +
> + unmap_mapping_range(gt->i915->drm.anon_inode->i_mapping,
> drm_vma_node_offset_addr(node) + vma_offset,
> vma->size,
> 1);
> + }
> }
> }
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 1c4576a4a5e9..bdd597200984 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -2722,18 +2722,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,
> @@ -2822,7 +2816,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,
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index b42651a387d9..bf2927e88e14 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2327,6 +2327,7 @@ int i915_gem_wait_for_idle(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_vma.c b/drivers/gpu/drm/i915/i915_vma.c
> index e0e677b2a3a9..677323644319 100644
> --- a/drivers/gpu/drm/i915/i915_vma.c
> +++ b/drivers/gpu/drm/i915/i915_vma.c
> @@ -861,7 +861,8 @@ 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;
> + struct i915_mmap_offset *mmo;
> u64 vma_offset;
>
> lockdep_assert_held(&vma->vm->mutex);
> @@ -873,10 +874,19 @@ 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);
> +
> + list_for_each_entry(mmo, &vma->obj->mmap_offsets, offset) {
> + node = &mmo->vma_node;
> + /* Only gtt-mmaps for this vma should be unmapped */
> + if (!drm_mm_node_allocated(&node->vm_node) ||
> + mmo->mmap_type != I915_MMAP_TYPE_GTT)
> + continue;
> +
> + 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)
> --
> 2.23.0
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
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: FW: [PATCH 1/5] drm/i915: Allow i915 to manage the vma offset nodes instead of drm core
[not found] ` <D5FCAD9A-8C9C-48ED-8ACD-40D24A572AF0@intel.com>
@ 2019-09-11 12:19 ` Chris Wilson
2019-09-19 13:05 ` Daniel Vetter
0 siblings, 1 reply; 28+ messages in thread
From: Chris Wilson @ 2019-09-11 12:19 UTC (permalink / raw)
To: intel-gfx
Quoting Balestrieri, Francesco (2019-09-11 13:03:25)
> On 04/09/2019, 13.33, "Intel-gfx on behalf of Daniel Vetter" <intel-gfx-bounces@lists.freedesktop.org on behalf of daniel@ffwll.ch> wrote:
>
> On Mon, Aug 26, 2019 at 2:21 PM Abdiel Janulgue
> > - ret = create_mmap_offset(obj);
> > - if (ret == 0)
> > - *offset = drm_vma_node_offset_addr(&obj->base.vma_node);
> > + mmo = kzalloc(sizeof(*mmo), GFP_KERNEL);
>
> I got thrown off a bunch of times here reading the code, but I think I
> got this right now.
>
> Why exactly do we want multiple vma offsets? Yes this makes it a
> drop-in replacement for the old cpu mmap ioctl, which was a bit
> dubious design. But if we go all new here, I really wonder about why
> this is necessary. No other discrete driver needs this, they all fix
> the mmap mode for the lifetime of an object, because flushing stuff is
> as expensive as just reallocating (or at least close enough).
>
> I think us going once again our separate route here needs a lot more
> justification than just "we've accidentally ended up with uapi like
> this 10 years ago".
That's exactly the whole point, to replace the uapi we accidentally
ended up with 10 years ago with the api that doesn't cause valgrind to
complain, is easily extensible and supports all legacy usecases which
should be a very good position to be in to support unknown future
usecases as well. Letting userspace control their mmapings is very
powerful, and we definitely do not want to be limiting their
flexibility.
That no other driver even seems to allow multiple mmaps, and so has
not developed a desire to manage multiple vma per object does not seem
to be a reason to limit ourselves. The infrastructure all supports it;
the only thing that is at odds is the desire to force the lowest common
denominator as the defacto standard.
-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: FW: [PATCH 1/5] drm/i915: Allow i915 to manage the vma offset nodes instead of drm core
2019-09-11 12:19 ` FW: " Chris Wilson
@ 2019-09-19 13:05 ` Daniel Vetter
2019-09-23 12:39 ` Daniel Vetter
0 siblings, 1 reply; 28+ messages in thread
From: Daniel Vetter @ 2019-09-19 13:05 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
On Wed, Sep 11, 2019 at 2:19 PM Chris Wilson <chris@chris-wilson.co.uk> wrote:
> Quoting Balestrieri, Francesco (2019-09-11 13:03:25)
> > On 04/09/2019, 13.33, "Intel-gfx on behalf of Daniel Vetter" <intel-gfx-bounces@lists.freedesktop.org on behalf of daniel@ffwll.ch> wrote:
> >
> > On Mon, Aug 26, 2019 at 2:21 PM Abdiel Janulgue
> > > - ret = create_mmap_offset(obj);
> > > - if (ret == 0)
> > > - *offset = drm_vma_node_offset_addr(&obj->base.vma_node);
> > > + mmo = kzalloc(sizeof(*mmo), GFP_KERNEL);
> >
> > I got thrown off a bunch of times here reading the code, but I think I
> > got this right now.
> >
> > Why exactly do we want multiple vma offsets? Yes this makes it a
> > drop-in replacement for the old cpu mmap ioctl, which was a bit
> > dubious design. But if we go all new here, I really wonder about why
> > this is necessary. No other discrete driver needs this, they all fix
> > the mmap mode for the lifetime of an object, because flushing stuff is
> > as expensive as just reallocating (or at least close enough).
> >
> > I think us going once again our separate route here needs a lot more
> > justification than just "we've accidentally ended up with uapi like
> > this 10 years ago".
>
> That's exactly the whole point, to replace the uapi we accidentally
> ended up with 10 years ago with the api that doesn't cause valgrind to
> complain, is easily extensible and supports all legacy usecases which
> should be a very good position to be in to support unknown future
> usecases as well. Letting userspace control their mmapings is very
> powerful, and we definitely do not want to be limiting their
> flexibility.
>
> That no other driver even seems to allow multiple mmaps, and so has
> not developed a desire to manage multiple vma per object does not seem
> to be a reason to limit ourselves. The infrastructure all supports it;
> the only thing that is at odds is the desire to force the lowest common
> denominator as the defacto standard.
Just because something is possible (or looks possible at first)
doesn't make it good uapi. That's how we get stuff like gtt mmap on
userptr bo, and then regrets. This entire thing here gets sold as
"uapi cleanup for lmem". Which makes sense, except most of the uapi
isn't really cleaned up at all:
- We still have relocations (and we even made them more powerful by
pipelining them, at a time when all our mesa drivers finally managed
to move to softpin). We could ditch all the reloc stuff for gen12+ and
lmem instead. Both amdgpu and nv50+ have done that.
- We're still adding more uapi that's all adding variable state.
Meanwhile everything else (gallium/iris, vk) move to invariant state
object models, where you create stuff once with the right properties
and call it a day. lmem does add quite a few new state bits here,
would be a lot simpler if we could make them as invariant as possible.
For placement we might need to allow changes to the priority, but not
to the placement list itself. For mmap, you just select the mmap mode
that you want (because once you fixing caching mode, placement list
and tiling bits, there's really not a choice anymore), and that's the
one you get with the single mmap offset.
Instead we just hang onto all the accumulated complexity, add more,
and the only cleanup I'm seeing is that we bake in the multi-mmap
model to save one single line in userspace for the valgrind
annotation. Which everyone has already anyway, that is not really
going to go away.
I think using lmem to clean up the uapi makes tons of sense, but if we
bother with that it should be a real cleanup. Not just cosmetics for
the one valgrind annotation line in userspace we have right now.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
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: FW: [PATCH 1/5] drm/i915: Allow i915 to manage the vma offset nodes instead of drm core
2019-09-19 13:05 ` Daniel Vetter
@ 2019-09-23 12:39 ` Daniel Vetter
0 siblings, 0 replies; 28+ messages in thread
From: Daniel Vetter @ 2019-09-23 12:39 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
On Thu, Sep 19, 2019 at 3:05 PM Daniel Vetter <daniel@ffwll.ch> wrote:
>
> On Wed, Sep 11, 2019 at 2:19 PM Chris Wilson <chris@chris-wilson.co.uk> wrote:
> > Quoting Balestrieri, Francesco (2019-09-11 13:03:25)
> > > On 04/09/2019, 13.33, "Intel-gfx on behalf of Daniel Vetter" <intel-gfx-bounces@lists.freedesktop.org on behalf of daniel@ffwll.ch> wrote:
> > >
> > > On Mon, Aug 26, 2019 at 2:21 PM Abdiel Janulgue
> > > > - ret = create_mmap_offset(obj);
> > > > - if (ret == 0)
> > > > - *offset = drm_vma_node_offset_addr(&obj->base.vma_node);
> > > > + mmo = kzalloc(sizeof(*mmo), GFP_KERNEL);
> > >
> > > I got thrown off a bunch of times here reading the code, but I think I
> > > got this right now.
> > >
> > > Why exactly do we want multiple vma offsets? Yes this makes it a
> > > drop-in replacement for the old cpu mmap ioctl, which was a bit
> > > dubious design. But if we go all new here, I really wonder about why
> > > this is necessary. No other discrete driver needs this, they all fix
> > > the mmap mode for the lifetime of an object, because flushing stuff is
> > > as expensive as just reallocating (or at least close enough).
> > >
> > > I think us going once again our separate route here needs a lot more
> > > justification than just "we've accidentally ended up with uapi like
> > > this 10 years ago".
> >
> > That's exactly the whole point, to replace the uapi we accidentally
> > ended up with 10 years ago with the api that doesn't cause valgrind to
> > complain, is easily extensible and supports all legacy usecases which
> > should be a very good position to be in to support unknown future
> > usecases as well. Letting userspace control their mmapings is very
> > powerful, and we definitely do not want to be limiting their
> > flexibility.
> >
> > That no other driver even seems to allow multiple mmaps, and so has
> > not developed a desire to manage multiple vma per object does not seem
> > to be a reason to limit ourselves. The infrastructure all supports it;
> > the only thing that is at odds is the desire to force the lowest common
> > denominator as the defacto standard.
>
> Just because something is possible (or looks possible at first)
> doesn't make it good uapi. That's how we get stuff like gtt mmap on
> userptr bo, and then regrets. This entire thing here gets sold as
> "uapi cleanup for lmem". Which makes sense, except most of the uapi
> isn't really cleaned up at all:
>
> - We still have relocations (and we even made them more powerful by
> pipelining them, at a time when all our mesa drivers finally managed
> to move to softpin). We could ditch all the reloc stuff for gen12+ and
> lmem instead. Both amdgpu and nv50+ have done that.
>
> - We're still adding more uapi that's all adding variable state.
> Meanwhile everything else (gallium/iris, vk) move to invariant state
> object models, where you create stuff once with the right properties
> and call it a day. lmem does add quite a few new state bits here,
> would be a lot simpler if we could make them as invariant as possible.
> For placement we might need to allow changes to the priority, but not
> to the placement list itself. For mmap, you just select the mmap mode
> that you want (because once you fixing caching mode, placement list
> and tiling bits, there's really not a choice anymore), and that's the
> one you get with the single mmap offset.
>
> Instead we just hang onto all the accumulated complexity, add more,
> and the only cleanup I'm seeing is that we bake in the multi-mmap
> model to save one single line in userspace for the valgrind
> annotation. Which everyone has already anyway, that is not really
> going to go away.
>
> I think using lmem to clean up the uapi makes tons of sense, but if we
> bother with that it should be a real cleanup. Not just cosmetics for
> the one valgrind annotation line in userspace we have right now.
Chatted with Chris, and one useful thing that multi-mmap gives us (and
which I ignored) is much easier testing of coherency between all the
different views. And we need that because the kernel needs that anyway
(at least cpu wc view against all others is needed for swap-out), and
we need to be able to test it well because our hw is historically
rather buggy in this area. So bummer and lots of sighs :-/
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
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 v2 1/5] drm/i915: Allow i915 to manage the vma offset nodes instead of drm core
2019-08-26 12:20 [PATCH 1/5] drm/i915: Allow i915 to manage the vma offset nodes instead of drm core Abdiel Janulgue
` (10 preceding siblings ...)
2019-09-04 10:33 ` [PATCH 1/5] " Daniel Vetter
@ 2019-10-07 8:48 ` Abdiel Janulgue
2019-10-07 8:48 ` [PATCH v2 2/5] drm/i915: define HAS_MAPPABLE_APERTURE Abdiel Janulgue
` (3 more replies)
2019-10-07 9:00 ` ✗ Fi.CI.BUILD: failure for series starting with [v2,5/5] drm/i915: Add cpu fault handler for mmap_offset (rev5) Patchwork
12 siblings, 4 replies; 28+ messages in thread
From: Abdiel Janulgue @ 2019-10-07 8:48 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
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 | 245 ++++++++++++++++--
drivers/gpu/drm/i915/gem/i915_gem_object.c | 13 +
drivers/gpu/drm/i915/gem/i915_gem_object.h | 9 +-
.../gpu/drm/i915/gem/i915_gem_object_types.h | 23 ++
.../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 | 2 +-
drivers/gpu/drm/i915/i915_vma.c | 24 +-
drivers/gpu/drm/i915/i915_vma.h | 4 +
12 files changed, 305 insertions(+), 49 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_domain.c b/drivers/gpu/drm/i915/gem/i915_gem_domain.c
index 9937b4c341f1..40792d2017a7 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_domain.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_domain.c
@@ -254,7 +254,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 fd4122d8c0a9..af4377a80502 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,12 @@ 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 */
+ mutex_lock(&priv->vma_lock);
+ vma->mmo = priv;
+ list_move(&vma->mmo_link, &priv->vma_list);
+ mutex_unlock(&priv->vma_lock);
+
if (CONFIG_DRM_I915_USERFAULT_AUTOSUSPEND)
intel_wakeref_auto(&i915->ggtt.userfault_wakeref,
msecs_to_jiffies_timeout(CONFIG_DRM_I915_USERFAULT_AUTOSUSPEND));
@@ -358,28 +365,29 @@ 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;
+ struct i915_mmap_offset *mmo;
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);
+
+ mutex_lock(&obj->mmo_lock);
+ list_for_each_entry(mmo, &obj->mmap_offsets, offset) {
+ 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);
for_each_ggtt_vma(vma, obj)
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 +395,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 +414,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 +430,66 @@ 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)
+static 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)
+{
+ INIT_LIST_HEAD(&mmo->vma_list);
+ mutex_init(&mmo->vma_lock);
+
+ 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,16 +497,23 @@ 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;
obj = i915_gem_object_lookup(file, handle);
@@ -459,10 +525,21 @@ i915_gem_mmap_gtt(struct drm_file *file,
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;
@@ -489,7 +566,125 @@ 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)
+{
+ struct i915_vma *vma, *vn;
+
+ 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);
+
+ mutex_lock(&mmo->vma_lock);
+ list_for_each_entry_safe(vma, vn, &mmo->vma_list, mmo_link)
+ vma->mmo = NULL;
+ mutex_unlock(&mmo->vma_lock);
+
+ 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 dbf9be9a79f4..8abcd5202374 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_object.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_object.c
@@ -58,6 +58,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;
@@ -155,6 +158,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)) {
@@ -180,6 +185,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 086a9bf5adcc..6473a87730ce 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_object.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_object.h
@@ -130,13 +130,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
@@ -356,7 +356,7 @@ 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
@@ -442,6 +442,9 @@ 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);
+
#define I915_PRIORITY_DISPLAY I915_USER_PRIORITY(I915_PRIORITY_MAX)
#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 c00b4f077f9e..74544eca5e2c 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
@@ -62,6 +62,24 @@ 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;
+ /* list of active VMA using this mmo for fencing */
+ struct list_head vma_list;
+ /* protects vma list */
+ struct mutex vma_lock;
+};
+
struct drm_i915_gem_object {
struct drm_gem_object base;
@@ -117,6 +135,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);
/*
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 cfa52c525691..23c2363dbb2d 100644
--- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c
+++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c
@@ -557,15 +557,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;
}
@@ -601,6 +606,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 resv, *hole;
+ /* Ownership transferred to parent gem object in create_mmap_offset */
+ struct i915_mmap_offset *mmo = kzalloc(sizeof(*mmo), GFP_KERNEL);
u64 hole_start, hole_end;
int loop, err;
@@ -644,9 +651,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 7b3d9d4517a0..d1090fcb6814 100644
--- a/drivers/gpu/drm/i915/gt/intel_reset.c
+++ b/drivers/gpu/drm/i915/gt/intel_reset.c
@@ -667,8 +667,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 9354924576c4..cc70aba6ac26 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -2644,18 +2644,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,
@@ -2744,7 +2738,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,
@@ -2755,7 +2748,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 b367467b03c2..fa6dd301a738 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2276,8 +2276,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);
@@ -2302,6 +2300,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 0ddbd3a5fb8d..e60ffae4b105 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -853,7 +853,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 9fdcd4e2c799..a95b2b79d806 100644
--- a/drivers/gpu/drm/i915/i915_vma.c
+++ b/drivers/gpu/drm/i915/i915_vma.c
@@ -130,6 +130,7 @@ vma_create(struct drm_i915_gem_object *obj,
}
INIT_LIST_HEAD(&vma->closed_link);
+ INIT_LIST_HEAD(&vma->mmo_link);
if (view && view->type != I915_GGTT_VIEW_NORMAL) {
vma->ggtt_view = *view;
@@ -1002,6 +1003,14 @@ void i915_vma_destroy(struct i915_vma *vma)
spin_unlock(&obj->vma.lock);
}
+ if (vma->mmo) {
+ struct i915_mmap_offset *mmo = vma->mmo;
+
+ mutex_lock(&mmo->vma_lock);
+ list_del(&vma->mmo_link);
+ mutex_unlock(&mmo->vma_lock);
+ }
+
__i915_vma_remove_closed(vma);
i915_vm_put(vma->vm);
@@ -1058,7 +1067,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);
@@ -1070,10 +1079,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 858908e3d1cc..34afd1a59ea5 100644
--- a/drivers/gpu/drm/i915/i915_vma.h
+++ b/drivers/gpu/drm/i915/i915_vma.h
@@ -63,6 +63,10 @@ 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;
+ struct list_head mmo_link;
+
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 v2 2/5] drm/i915: define HAS_MAPPABLE_APERTURE
2019-10-07 8:48 ` [PATCH v2 " Abdiel Janulgue
@ 2019-10-07 8:48 ` Abdiel Janulgue
2019-10-07 8:48 ` [PATCH v2 3/5] drm/i915: Introduce DRM_I915_GEM_MMAP_OFFSET Abdiel Janulgue
` (2 subsequent siblings)
3 siblings, 0 replies; 28+ messages in thread
From: Abdiel Janulgue @ 2019-10-07 8:48 UTC (permalink / raw)
To: intel-gfx; +Cc: Matthew Auld
From: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
The following patches in the series will use it to avoid certain
operations when aperture is not available in HW.
Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Matthew Auld <matthew.auld@intel.com>
---
drivers/gpu/drm/i915/i915_drv.h | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index fa6dd301a738..659319f03f18 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2080,6 +2080,8 @@ IS_SUBPLATFORM(const struct drm_i915_private *i915,
#define OVERLAY_NEEDS_PHYSICAL(dev_priv) \
(INTEL_INFO(dev_priv)->display.overlay_needs_physical)
+#define HAS_MAPPABLE_APERTURE(dev_priv) (dev_priv->ggtt.mappable_end > 0)
+
/* Early gen2 have a totally busted CS tlb and require pinned batches. */
#define HAS_BROKEN_CS_TLB(dev_priv) (IS_I830(dev_priv) || IS_I845G(dev_priv))
--
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 v2 3/5] drm/i915: Introduce DRM_I915_GEM_MMAP_OFFSET
2019-10-07 8:48 ` [PATCH v2 " Abdiel Janulgue
2019-10-07 8:48 ` [PATCH v2 2/5] drm/i915: define HAS_MAPPABLE_APERTURE Abdiel Janulgue
@ 2019-10-07 8:48 ` Abdiel Janulgue
2019-10-07 8:48 ` [PATCH v2 4/5] drm/i915: cpu-map based dumb buffers Abdiel Janulgue
2019-10-07 8:48 ` [PATCH v2 5/5] drm/i915: Add cpu fault handler for mmap_offset Abdiel Janulgue
3 siblings, 0 replies; 28+ messages in thread
From: Abdiel Janulgue @ 2019-10-07 8:48 UTC (permalink / raw)
To: intel-gfx; +Cc: Matthew Auld
This is really just an alias of mmap_gtt. Add a new CPU mmap
implementation that allows multiple 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.
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 | 36 +++++++++++++++++--
.../gpu/drm/i915/gem/i915_gem_object_types.h | 3 ++
drivers/gpu/drm/i915/i915_drv.c | 2 +-
include/uapi/drm/i915_drm.h | 28 +++++++++++++++
4 files changed, 66 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_mman.c b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
index af4377a80502..ae1e687408da 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_mman.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
@@ -144,6 +144,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
@@ -171,7 +174,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
@@ -545,6 +548,27 @@ __assign_gem_object_mmap_data(struct drm_file *file,
return ret;
}
+static int gem_mmap_offset(struct drm_device *dev, void *data,
+ struct drm_file *file)
+{
+ struct drm_i915_gem_mmap_offset *args = data;
+ enum i915_mmap_type type;
+
+ if ((args->flags & (I915_MMAP_OFFSET_WC | I915_MMAP_OFFSET_WB)) &&
+ !boot_cpu_has(X86_FEATURE_PAT))
+ return -ENODEV;
+
+ if (args->flags & I915_MMAP_OFFSET_WC)
+ type = I915_MMAP_TYPE_OFFSET_WC;
+ else if (args->flags & I915_MMAP_OFFSET_WB)
+ type = I915_MMAP_TYPE_OFFSET_WB;
+ else if (args->flags & I915_MMAP_OFFSET_UC)
+ type = I915_MMAP_TYPE_OFFSET_UC;
+
+ return __assign_gem_object_mmap_data(file, args->handle, type,
+ &args->offset);
+}
+
/**
* i915_gem_mmap_gtt_ioctl - prepare an object for GTT mmap'ing
* @dev: DRM device
@@ -564,7 +588,15 @@ int
i915_gem_mmap_gtt_ioctl(struct drm_device *dev, void *data,
struct drm_file *file)
{
- struct drm_i915_gem_mmap_gtt *args = data;
+ struct drm_i915_gem_mmap_offset *args = data;
+ struct drm_i915_private *i915 = to_i915(dev);
+
+ if (args->flags & I915_MMAP_OFFSET_FLAGS)
+ return gem_mmap_offset(dev, data, file);
+
+ if (!HAS_MAPPABLE_APERTURE(i915))
+ /* No aperture, cannot mmap via legacy GTT */
+ return -ENODEV;
return __assign_gem_object_mmap_data(file, args->handle,
I915_MMAP_TYPE_GTT,
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 74544eca5e2c..18e891f4e9d3 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
@@ -64,6 +64,9 @@ struct drm_i915_gem_object_ops {
enum i915_mmap_type {
I915_MMAP_TYPE_GTT = 0,
+ I915_MMAP_TYPE_OFFSET_WC,
+ I915_MMAP_TYPE_OFFSET_WB,
+ I915_MMAP_TYPE_OFFSET_UC,
};
struct i915_mmap_offset {
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index cc70aba6ac26..9182da57182b 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -2696,7 +2696,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_gtt_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/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index 30c542144016..bc85656ab7fd 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -359,6 +359,7 @@ typedef struct _drm_i915_sarea {
#define DRM_I915_QUERY 0x39
#define DRM_I915_GEM_VM_CREATE 0x3a
#define DRM_I915_GEM_VM_DESTROY 0x3b
+#define DRM_I915_GEM_MMAP_OFFSET DRM_I915_GEM_MMAP_GTT
/* Must be kept compact -- no holes */
#define DRM_IOCTL_I915_INIT DRM_IOW( DRM_COMMAND_BASE + DRM_I915_INIT, drm_i915_init_t)
@@ -421,6 +422,7 @@ typedef struct _drm_i915_sarea {
#define DRM_IOCTL_I915_QUERY DRM_IOWR(DRM_COMMAND_BASE + DRM_I915_QUERY, struct drm_i915_query)
#define DRM_IOCTL_I915_GEM_VM_CREATE DRM_IOWR(DRM_COMMAND_BASE + DRM_I915_GEM_VM_CREATE, struct drm_i915_gem_vm_control)
#define DRM_IOCTL_I915_GEM_VM_DESTROY DRM_IOW (DRM_COMMAND_BASE + DRM_I915_GEM_VM_DESTROY, struct drm_i915_gem_vm_control)
+#define DRM_IOCTL_I915_GEM_MMAP_OFFSET DRM_IOWR(DRM_COMMAND_BASE + DRM_I915_GEM_MMAP_OFFSET, struct drm_i915_gem_mmap_offset)
/* Allow drivers to submit batchbuffers directly to hardware, relying
* on the security mechanisms provided by hardware.
@@ -611,6 +613,7 @@ typedef struct drm_i915_irq_wait {
* See I915_EXEC_FENCE_OUT and I915_EXEC_FENCE_SUBMIT.
*/
#define I915_PARAM_HAS_EXEC_SUBMIT_FENCE 53
+
/* Must be kept compact -- no holes and well documented */
typedef struct drm_i915_getparam {
@@ -786,6 +789,31 @@ 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 _WC/_WB flags
+ * should be passed here.
+ */
+ __u64 flags;
+#define I915_MMAP_OFFSET_WC (1 << 0)
+#define I915_MMAP_OFFSET_WB (1 << 1)
+#define I915_MMAP_OFFSET_UC (1 << 2)
+#define I915_MMAP_OFFSET_FLAGS \
+ (I915_MMAP_OFFSET_WC | I915_MMAP_OFFSET_WB | I915_MMAP_OFFSET_UC)
+};
+
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 v2 4/5] drm/i915: cpu-map based dumb buffers
2019-10-07 8:48 ` [PATCH v2 " Abdiel Janulgue
2019-10-07 8:48 ` [PATCH v2 2/5] drm/i915: define HAS_MAPPABLE_APERTURE Abdiel Janulgue
2019-10-07 8:48 ` [PATCH v2 3/5] drm/i915: Introduce DRM_I915_GEM_MMAP_OFFSET Abdiel Janulgue
@ 2019-10-07 8:48 ` Abdiel Janulgue
2019-10-07 8:48 ` [PATCH v2 5/5] drm/i915: Add cpu fault handler for mmap_offset Abdiel Janulgue
3 siblings, 0 replies; 28+ messages in thread
From: Abdiel Janulgue @ 2019-10-07 8:48 UTC (permalink / raw)
To: intel-gfx; +Cc: Matthew Auld
If there is no aperture we can't use map_gtt to map dumb buffers, so we
need a cpu-map based path to do it. We prefer map_gtt on platforms that
do have aperture.
Signed-off-by: Abdiel Janulgue <abdiel.janulgue@linux.intel.com>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
Cc: Matthew Auld <matthew.auld@intel.com>
---
drivers/gpu/drm/i915/gem/i915_gem_mman.c | 13 +++++++++++++
drivers/gpu/drm/i915/i915_drv.c | 1 +
drivers/gpu/drm/i915/i915_drv.h | 2 ++
3 files changed, 16 insertions(+)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_mman.c b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
index ae1e687408da..a9604d0db606 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_mman.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
@@ -569,6 +569,19 @@ static int gem_mmap_offset(struct drm_device *dev, void *data,
&args->offset);
}
+int
+i915_gem_mmap_dumb(struct drm_file *file,
+ struct drm_device *dev,
+ u32 handle,
+ u64 *offset)
+{
+ struct drm_i915_private *i915 = dev->dev_private;
+ enum i915_mmap_type mmap_type = HAS_MAPPABLE_APERTURE(i915) ?
+ I915_MMAP_TYPE_GTT : I915_MMAP_TYPE_OFFSET_WC;
+
+ return __assign_gem_object_mmap_data(file, handle, mmap_type, offset);
+}
+
/**
* i915_gem_mmap_gtt_ioctl - prepare an object for GTT mmap'ing
* @dev: DRM device
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 9182da57182b..0fbbe03befb0 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -2748,6 +2748,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,
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 659319f03f18..47487beaf0e3 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2278,6 +2278,8 @@ 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_dumb(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);
--
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 v2 5/5] drm/i915: Add cpu fault handler for mmap_offset
2019-10-07 8:48 ` [PATCH v2 " Abdiel Janulgue
` (2 preceding siblings ...)
2019-10-07 8:48 ` [PATCH v2 4/5] drm/i915: cpu-map based dumb buffers Abdiel Janulgue
@ 2019-10-07 8:48 ` Abdiel Janulgue
3 siblings, 0 replies; 28+ messages in thread
From: Abdiel Janulgue @ 2019-10-07 8:48 UTC (permalink / raw)
To: intel-gfx; +Cc: Matthew Auld
Fault handler to handle missing pages for shmem-backed objects.
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 | 125 ++++++++++++++++++-----
1 file changed, 100 insertions(+), 25 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_mman.c b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
index a9604d0db606..02afdae812ba 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"
@@ -200,6 +201,67 @@ 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 size = area->vm_end - area->vm_start;
+ bool write = area->vm_flags & VM_WRITE;
+ int i, ret;
+
+ /* 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_ERROR)
+ break;
+ }
+
+ i915_gem_object_unpin_pages(obj);
+
+ return vmf_ret;
+}
+
/**
* i915_gem_fault - fault a page into the GTT
* @vmf: fault info
@@ -342,30 +404,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)
@@ -660,6 +699,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_OFFSET_WC:
+ vma->vm_page_prot =
+ pgprot_writecombine(vm_get_page_prot(vma->vm_flags));
+ break;
+ case I915_MMAP_TYPE_OFFSET_WB:
+ vma->vm_page_prot = vm_get_page_prot(vma->vm_flags);
+ break;
+ case I915_MMAP_TYPE_OFFSET_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
@@ -727,7 +793,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_OFFSET_WC:
+ case I915_MMAP_TYPE_OFFSET_WB:
+ case I915_MMAP_TYPE_OFFSET_UC:
+ set_vmdata_mmap_offset(mmo, vma);
+ break;
+ case I915_MMAP_TYPE_GTT:
+ vma->vm_ops = &i915_gem_gtt_vm_ops;
+ break;
+ }
return 0;
}
--
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
* ✗ Fi.CI.BUILD: failure for series starting with [v2,5/5] drm/i915: Add cpu fault handler for mmap_offset (rev5)
2019-08-26 12:20 [PATCH 1/5] drm/i915: Allow i915 to manage the vma offset nodes instead of drm core Abdiel Janulgue
` (11 preceding siblings ...)
2019-10-07 8:48 ` [PATCH v2 " Abdiel Janulgue
@ 2019-10-07 9:00 ` Patchwork
12 siblings, 0 replies; 28+ messages in thread
From: Patchwork @ 2019-10-07 9:00 UTC (permalink / raw)
To: Abdiel Janulgue; +Cc: intel-gfx
== Series Details ==
Series: series starting with [v2,5/5] drm/i915: Add cpu fault handler for mmap_offset (rev5)
URL : https://patchwork.freedesktop.org/series/65793/
State : failure
== Summary ==
Applying: drm/i915: Add cpu fault handler for mmap_offset
error: sha1 information is lacking or useless (drivers/gpu/drm/i915/gem/i915_gem_mman.c).
error: could not build fake ancestor
hint: Use 'git am --show-current-patch' to see the failed patch
Patch failed at 0001 drm/i915: Add cpu fault handler for mmap_offset
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
_______________________________________________
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/5] drm/i915: Allow i915 to manage the vma offset nodes instead of drm core
@ 2019-10-23 8:30 Abdiel Janulgue
0 siblings, 0 replies; 28+ messages in thread
From: Abdiel Janulgue @ 2019-10-23 8:30 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
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 | 229 ++++++++++++++++--
drivers/gpu/drm/i915/gem/i915_gem_object.c | 13 +
drivers/gpu/drm/i915/gem/i915_gem_object.h | 9 +-
.../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 | 2 +-
drivers/gpu/drm/i915/i915_vma.c | 15 +-
drivers/gpu/drm/i915/i915_vma.h | 3 +
12 files changed, 274 insertions(+), 50 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_domain.c b/drivers/gpu/drm/i915/gem/i915_gem_domain.c
index 9937b4c341f1..40792d2017a7 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_domain.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_domain.c
@@ -254,7 +254,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 fd4122d8c0a9..3491bb06606b 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 (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)
+static 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,16 +486,23 @@ 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;
obj = i915_gem_object_lookup(file, handle);
@@ -459,10 +514,21 @@ i915_gem_mmap_gtt(struct drm_file *file,
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;
@@ -489,7 +555,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 a50296cce0d8..a44b9834794e 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_object.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_object.c
@@ -59,6 +59,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;
@@ -156,6 +159,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)) {
@@ -181,6 +186,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 aead7e6725f9..ef409897612d 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
@@ -376,7 +376,7 @@ 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
@@ -462,6 +462,9 @@ 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);
+
#define I915_PRIORITY_DISPLAY I915_USER_PRIORITY(I915_PRIORITY_MAX)
#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 a387e3ee728b..c5c305bd9927 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
@@ -62,6 +62,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;
@@ -117,6 +131,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 d45a93928ff5..01d1aa5ed8c3 100644
--- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c
+++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c
@@ -557,15 +557,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;
}
@@ -601,6 +606,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 resv, *hole;
+ /* Ownership transferred to parent gem object in create_mmap_offset */
+ struct i915_mmap_offset *mmo = kzalloc(sizeof(*mmo), GFP_KERNEL);
u64 hole_start, hole_end;
int loop, err;
@@ -644,9 +651,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 bf8d1ed4b1d8..5e985c0cb949 100644
--- a/drivers/gpu/drm/i915/gt/intel_reset.c
+++ b/drivers/gpu/drm/i915/gt/intel_reset.c
@@ -667,8 +667,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 5138d1eed306..7dba9b2ea00b 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -2646,18 +2646,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,
@@ -2746,7 +2740,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,
@@ -2757,7 +2750,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 8882c0908c3b..ed0fc1d6dfab 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1932,8 +1932,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);
@@ -1958,6 +1956,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 b882988056bd..4fece1c5e5c0 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -853,7 +853,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 d733bcf262f0..98d4545a3320 100644
--- a/drivers/gpu/drm/i915/i915_vma.c
+++ b/drivers/gpu/drm/i915/i915_vma.c
@@ -1058,7 +1058,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);
@@ -1070,10 +1070,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
* [PATCH 1/5] drm/i915: Allow i915 to manage the vma offset nodes instead of drm core
@ 2019-10-23 8:24 Abdiel Janulgue
0 siblings, 0 replies; 28+ messages in thread
From: Abdiel Janulgue @ 2019-10-23 8:24 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
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 | 229 ++++++++++++++++--
drivers/gpu/drm/i915/gem/i915_gem_object.c | 13 +
drivers/gpu/drm/i915/gem/i915_gem_object.h | 9 +-
.../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 | 2 +-
drivers/gpu/drm/i915/i915_vma.c | 15 +-
drivers/gpu/drm/i915/i915_vma.h | 3 +
12 files changed, 274 insertions(+), 50 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_domain.c b/drivers/gpu/drm/i915/gem/i915_gem_domain.c
index 9937b4c341f1..40792d2017a7 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_domain.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_domain.c
@@ -254,7 +254,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 fd4122d8c0a9..3491bb06606b 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 (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)
+static 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,16 +486,23 @@ 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;
obj = i915_gem_object_lookup(file, handle);
@@ -459,10 +514,21 @@ i915_gem_mmap_gtt(struct drm_file *file,
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;
@@ -489,7 +555,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 a50296cce0d8..a44b9834794e 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_object.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_object.c
@@ -59,6 +59,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;
@@ -156,6 +159,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)) {
@@ -181,6 +186,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 aead7e6725f9..ef409897612d 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
@@ -376,7 +376,7 @@ 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
@@ -462,6 +462,9 @@ 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);
+
#define I915_PRIORITY_DISPLAY I915_USER_PRIORITY(I915_PRIORITY_MAX)
#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 a387e3ee728b..c5c305bd9927 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
@@ -62,6 +62,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;
@@ -117,6 +131,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 d45a93928ff5..01d1aa5ed8c3 100644
--- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c
+++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c
@@ -557,15 +557,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;
}
@@ -601,6 +606,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 resv, *hole;
+ /* Ownership transferred to parent gem object in create_mmap_offset */
+ struct i915_mmap_offset *mmo = kzalloc(sizeof(*mmo), GFP_KERNEL);
u64 hole_start, hole_end;
int loop, err;
@@ -644,9 +651,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 bf8d1ed4b1d8..5e985c0cb949 100644
--- a/drivers/gpu/drm/i915/gt/intel_reset.c
+++ b/drivers/gpu/drm/i915/gt/intel_reset.c
@@ -667,8 +667,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 5138d1eed306..7dba9b2ea00b 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -2646,18 +2646,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,
@@ -2746,7 +2740,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,
@@ -2757,7 +2750,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 8882c0908c3b..ed0fc1d6dfab 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1932,8 +1932,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);
@@ -1958,6 +1956,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 b882988056bd..4fece1c5e5c0 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -853,7 +853,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 d733bcf262f0..98d4545a3320 100644
--- a/drivers/gpu/drm/i915/i915_vma.c
+++ b/drivers/gpu/drm/i915/i915_vma.c
@@ -1058,7 +1058,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);
@@ -1070,10 +1070,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
* [PATCH 1/5] drm/i915: Allow i915 to manage the vma offset nodes instead of drm core
@ 2019-10-21 10:48 Abdiel Janulgue
0 siblings, 0 replies; 28+ messages in thread
From: Abdiel Janulgue @ 2019-10-21 10:48 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
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 | 229 ++++++++++++++++--
drivers/gpu/drm/i915/gem/i915_gem_object.c | 13 +
drivers/gpu/drm/i915/gem/i915_gem_object.h | 9 +-
.../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 | 2 +-
drivers/gpu/drm/i915/i915_vma.c | 15 +-
drivers/gpu/drm/i915/i915_vma.h | 4 +
12 files changed, 275 insertions(+), 50 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_domain.c b/drivers/gpu/drm/i915/gem/i915_gem_domain.c
index 9937b4c341f1..40792d2017a7 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_domain.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_domain.c
@@ -254,7 +254,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 fd4122d8c0a9..3491bb06606b 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 (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)
+static 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,16 +486,23 @@ 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;
obj = i915_gem_object_lookup(file, handle);
@@ -459,10 +514,21 @@ i915_gem_mmap_gtt(struct drm_file *file,
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;
@@ -489,7 +555,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 dbf9be9a79f4..8abcd5202374 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_object.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_object.c
@@ -58,6 +58,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;
@@ -155,6 +158,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)) {
@@ -180,6 +185,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 85921796851f..d529d011e85c 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_object.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_object.h
@@ -131,13 +131,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
@@ -375,7 +375,7 @@ 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
@@ -461,6 +461,9 @@ 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);
+
#define I915_PRIORITY_DISPLAY I915_USER_PRIORITY(I915_PRIORITY_MAX)
#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 a387e3ee728b..c5c305bd9927 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
@@ -62,6 +62,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;
@@ -117,6 +131,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 65d4dbf91999..a4b14a2fb77a 100644
--- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c
+++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c
@@ -557,15 +557,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;
}
@@ -601,6 +606,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 resv, *hole;
+ /* Ownership transferred to parent gem object in create_mmap_offset */
+ struct i915_mmap_offset *mmo = kzalloc(sizeof(*mmo), GFP_KERNEL);
u64 hole_start, hole_end;
int loop, err;
@@ -644,9 +651,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 bf8d1ed4b1d8..5e985c0cb949 100644
--- a/drivers/gpu/drm/i915/gt/intel_reset.c
+++ b/drivers/gpu/drm/i915/gt/intel_reset.c
@@ -667,8 +667,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 21b36a6143b9..ead2e6ca7907 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -2648,18 +2648,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,
@@ -2748,7 +2742,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,
@@ -2759,7 +2752,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 8882c0908c3b..ed0fc1d6dfab 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1932,8 +1932,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);
@@ -1958,6 +1956,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 a2ba123f7b31..2e20c6020334 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -852,7 +852,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 e90c4d0af8fd..c89e7b7a797b 100644
--- a/drivers/gpu/drm/i915/i915_vma.c
+++ b/drivers/gpu/drm/i915/i915_vma.c
@@ -1058,7 +1058,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);
@@ -1070,10 +1070,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 858908e3d1cc..34afd1a59ea5 100644
--- a/drivers/gpu/drm/i915/i915_vma.h
+++ b/drivers/gpu/drm/i915/i915_vma.h
@@ -63,6 +63,10 @@ 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;
+ struct list_head mmo_link;
+
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-10-23 8:30 UTC | newest]
Thread overview: 28+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-26 12:20 [PATCH 1/5] drm/i915: Allow i915 to manage the vma offset nodes instead of drm core Abdiel Janulgue
2019-08-26 12:20 ` [PATCH 2/5] drm/i915: Introduce DRM_I915_GEM_MMAP_OFFSET Abdiel Janulgue
2019-08-26 12:21 ` [PATCH 3/5] drm/i915: Add cpu fault handler for mmap_offset Abdiel Janulgue
2019-08-26 12:44 ` Chris Wilson
2019-08-26 12:46 ` Chris Wilson
2019-08-26 12:21 ` [PATCH 4/5] drm/i915: define HAS_MAPPABLE_APERTURE Abdiel Janulgue
2019-08-26 12:21 ` [PATCH 5/5] drm/i915: cpu-map based dumb buffers Abdiel Janulgue
2019-08-26 12:48 ` Chris Wilson
2019-08-27 8:31 ` Abdiel Janulgue
2019-08-26 12:42 ` [PATCH 1/5] drm/i915: Allow i915 to manage the vma offset nodes instead of drm core Chris Wilson
2019-08-26 12:51 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/5] " Patchwork
2019-08-26 12:52 ` ✗ Fi.CI.SPARSE: " Patchwork
2019-08-26 12:53 ` [PATCH 1/5] " Chris Wilson
2019-08-26 13:36 ` ✓ Fi.CI.BAT: success for series starting with [1/5] " Patchwork
2019-08-26 15:00 ` ✗ Fi.CI.IGT: failure " Patchwork
2019-09-04 10:33 ` [PATCH 1/5] " Daniel Vetter
[not found] ` <D5FCAD9A-8C9C-48ED-8ACD-40D24A572AF0@intel.com>
2019-09-11 12:19 ` FW: " Chris Wilson
2019-09-19 13:05 ` Daniel Vetter
2019-09-23 12:39 ` Daniel Vetter
2019-10-07 8:48 ` [PATCH v2 " Abdiel Janulgue
2019-10-07 8:48 ` [PATCH v2 2/5] drm/i915: define HAS_MAPPABLE_APERTURE Abdiel Janulgue
2019-10-07 8:48 ` [PATCH v2 3/5] drm/i915: Introduce DRM_I915_GEM_MMAP_OFFSET Abdiel Janulgue
2019-10-07 8:48 ` [PATCH v2 4/5] drm/i915: cpu-map based dumb buffers Abdiel Janulgue
2019-10-07 8:48 ` [PATCH v2 5/5] drm/i915: Add cpu fault handler for mmap_offset Abdiel Janulgue
2019-10-07 9:00 ` ✗ Fi.CI.BUILD: failure for series starting with [v2,5/5] drm/i915: Add cpu fault handler for mmap_offset (rev5) Patchwork
2019-10-21 10:48 [PATCH 1/5] drm/i915: Allow i915 to manage the vma offset nodes instead of drm core Abdiel Janulgue
2019-10-23 8:24 Abdiel Janulgue
2019-10-23 8:30 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.