[PATCH 4/8] drm/i915: Drop PRIME exported GEM objects on driver remove
Janusz Krzysztofik
janusz.krzysztofik at linux.intel.com
Wed Jul 1 10:43:30 UTC 2020
PRIME exported GEM objects have their own file descriptors which can
live longer than device file descriptors under which they were created
and exported. Since those objects keep references to their associated
address spaces, DMA API may issue a warning on device associated DMA
mappings still active when the device with those objects still open is
removed. Moreover, subsequent attempts to release those address spaces
and revoke their associated DMA mappings spaces while closing those
file descriptors may be judged by intel-iommu code as bugs and result
in kernel panic.
Since exported GEM objects become useless after the device is no longer
available, drop them and their associations with dma_buf structures on
device removal. For that to be possible, maintain a list of actually
exported objects. Let our .prime_export() DRM driver callback add
exported objects to this list and implement our own .release() dma_buf
operation which does the opposite. Flush the list on device removal.
Also, protect our dma_buf operations from dereferencing objects which
no longer exist.
<4> [36.900985] ------------[ cut here ]------------
<2> [36.901005] kernel BUG at drivers/iommu/intel-iommu.c:3717!
<4> [36.901105] invalid opcode: 0000 [#1] PREEMPT SMP NOPTI
<4> [36.901117] CPU: 0 PID: 39 Comm: kworker/u8:1 Tainted: G U W 5.7.0-rc5-CI-CI_DRM_8485+ #1
<4> [36.901133] Hardware name: Intel Corporation Elkhart Lake Embedded Platform/ElkhartLake LPDDR4x T3 CRB, BIOS EHLSFWI1.R00.1484.A00.1911290833 11/29/2019
<4> [36.901250] Workqueue: i915 __i915_vm_release [i915]
<4> [36.901264] RIP: 0010:intel_unmap+0x1f5/0x230
<4> [36.901274] Code: 01 e8 9f bc a9 ff 85 c0 74 09 80 3d df 60 09 01 00 74 19 65 ff 0d 13 12 97 7e 0f 85 fc fe ff ff e8 82 b0 95 ff e9 f2 fe ff ff <0f> 0b e8 d4 bd a9 ff 85 c0 75 de 48 c7 c2 10 84 2c 82 be 54 00 00
<4> [36.901302] RSP: 0018:ffffc900001ebdc0 EFLAGS: 00010246
<4> [36.901313] RAX: 0000000000000000 RBX: ffff8882561dd000 RCX: 0000000000000000
<4> [36.901324] RDX: 0000000000001000 RSI: 00000000ffd9c000 RDI: ffff888274c94000
<4> [36.901336] RBP: ffff888274c940b0 R08: 0000000000000000 R09: 0000000000000001
<4> [36.901348] R10: 000000000a25d812 R11: 00000000112af2d4 R12: ffff888252c70200
<4> [36.901360] R13: 00000000ffd9c000 R14: 0000000000001000 R15: ffff8882561dd010
<4> [36.901372] FS: 0000000000000000(0000) GS:ffff888278000000(0000) knlGS:0000000000000000
<4> [36.901386] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
<4> [36.901396] CR2: 00007f06def54950 CR3: 0000000255844000 CR4: 0000000000340ef0
<4> [36.901408] Call Trace:
<4> [36.901418] ? process_one_work+0x1de/0x600
<4> [36.901494] cleanup_page_dma+0x37/0x70 [i915]
<4> [36.901573] free_pd+0x9/0x20 [i915]
<4> [36.901644] gen8_ppgtt_cleanup+0x59/0xc0 [i915]
<4> [36.901721] __i915_vm_release+0x14/0x30 [i915]
<4> [36.901733] process_one_work+0x268/0x600
<4> [36.901744] ? __schedule+0x307/0x8d0
<4> [36.901756] worker_thread+0x37/0x380
<4> [36.901766] ? process_one_work+0x600/0x600
<4> [36.901775] kthread+0x140/0x160
<4> [36.901783] ? kthread_park+0x80/0x80
<4> [36.901792] ret_from_fork+0x24/0x50
<4> [36.901804] Modules linked in: mei_hdcp i915 x86_pkg_temp_thermal coretemp crct10dif_pclmul crc32_pclmul ghash_clmulni_intel ax88179_178a usbnet mii mei_me mei prime_numbers intel_lpss_pci
<4> [36.901857] ---[ end trace 52d1b4d81f8d1ea7 ]---
Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik at linux.intel.com>
---
drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c | 194 +++++++++++++++---
drivers/gpu/drm/i915/i915_drv.h | 7 +
drivers/gpu/drm/i915/i915_gem.c | 4 +
.../gpu/drm/i915/selftests/mock_gem_device.c | 2 +
4 files changed, 176 insertions(+), 31 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c b/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c
index 2679380159fc..8952ce7cf768 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c
@@ -12,19 +12,39 @@
#include "i915_gem_object.h"
#include "i915_scatterlist.h"
-static struct drm_i915_gem_object *dma_buf_to_obj(struct dma_buf *buf)
+struct i915_dmabuf_priv {
+ struct drm_i915_private *i915;
+ struct drm_i915_gem_object *obj;
+ spinlock_t lock;
+ struct list_head link;
+};
+
+static struct drm_i915_gem_object *dmabuf_get_obj(struct i915_dmabuf_priv *priv)
{
- return to_intel_bo(buf->priv);
+ struct drm_i915_gem_object *obj;
+
+ spin_lock(&priv->lock);
+ obj = priv->obj;
+ if (obj)
+ i915_gem_object_get(obj);
+ spin_unlock(&priv->lock);
+
+ return obj;
}
static struct sg_table *i915_gem_map_dma_buf(struct dma_buf_attachment *attachment,
enum dma_data_direction dir)
{
- struct drm_i915_gem_object *obj = dma_buf_to_obj(attachment->dmabuf);
+ struct dma_buf *dma_buf = attachment->dmabuf;
+ struct drm_i915_gem_object *obj;
struct sg_table *st;
struct scatterlist *src, *dst;
int ret, i;
+ obj = dmabuf_get_obj(dma_buf->priv);
+ if (!obj)
+ return NULL;
+
ret = i915_gem_object_pin_pages(obj);
if (ret)
goto err;
@@ -64,6 +84,7 @@ static struct sg_table *i915_gem_map_dma_buf(struct dma_buf_attachment *attachme
err_unpin_pages:
i915_gem_object_unpin_pages(obj);
err:
+ i915_gem_object_put(obj);
return ERR_PTR(ret);
}
@@ -71,7 +92,7 @@ static void i915_gem_unmap_dma_buf(struct dma_buf_attachment *attachment,
struct sg_table *sg,
enum dma_data_direction dir)
{
- struct drm_i915_gem_object *obj = dma_buf_to_obj(attachment->dmabuf);
+ struct drm_i915_gem_object *obj;
dma_unmap_sg_attrs(attachment->dev,
sg->sgl, sg->nents, dir,
@@ -79,92 +100,155 @@ static void i915_gem_unmap_dma_buf(struct dma_buf_attachment *attachment,
sg_free_table(sg);
kfree(sg);
- i915_gem_object_unpin_pages(obj);
+ obj = dmabuf_get_obj(attachment->dmabuf->priv);
+ if (obj) {
+ i915_gem_object_unpin_pages(obj);
+ i915_gem_object_put(obj);
+ }
+}
+
+static void i915_gem_dmabuf_remove(struct i915_dmabuf_priv *dmabuf_priv)
+{
+ struct drm_i915_gem_object *obj;
+
+ if (!list_empty(&dmabuf_priv->link))
+ list_del_init(&dmabuf_priv->link);
+
+ spin_lock(&dmabuf_priv->lock);
+ obj = xchg(&dmabuf_priv->obj, NULL);
+ spin_unlock(&dmabuf_priv->lock);
+
+ if (obj)
+ i915_gem_object_put(obj);
+}
+
+static void i915_gem_dmabuf_release(struct dma_buf *dma_buf)
+{
+ struct i915_dmabuf_priv *dmabuf_priv = dma_buf->priv;
+ struct drm_i915_private *i915 = dmabuf_priv->i915;
+
+ mutex_lock(&i915->gem.dmabuf.lock);
+ i915_gem_dmabuf_remove(dmabuf_priv);
+ mutex_unlock(&i915->gem.dmabuf.lock);
+
+ kfree(dmabuf_priv);
+
+ drm_dev_put(&i915->drm);
}
static void *i915_gem_dmabuf_vmap(struct dma_buf *dma_buf)
{
- struct drm_i915_gem_object *obj = dma_buf_to_obj(dma_buf);
+ struct drm_i915_gem_object *obj = dmabuf_get_obj(dma_buf->priv);
+ void *ptr;
+
+ if (!obj)
+ return NULL;
- return i915_gem_object_pin_map(obj, I915_MAP_WB);
+ ptr = i915_gem_object_pin_map(obj, I915_MAP_WB);
+ i915_gem_object_put(obj);
+
+ return ptr;
}
static void i915_gem_dmabuf_vunmap(struct dma_buf *dma_buf, void *vaddr)
{
- struct drm_i915_gem_object *obj = dma_buf_to_obj(dma_buf);
+ struct drm_i915_gem_object *obj = dmabuf_get_obj(dma_buf->priv);
+
+ if (!obj)
+ return;
i915_gem_object_flush_map(obj);
i915_gem_object_unpin_map(obj);
+ i915_gem_object_put(obj);
}
static int i915_gem_dmabuf_mmap(struct dma_buf *dma_buf, struct vm_area_struct *vma)
{
- struct drm_i915_gem_object *obj = dma_buf_to_obj(dma_buf);
+ struct drm_i915_gem_object *obj = dmabuf_get_obj(dma_buf->priv);
int ret;
- if (obj->base.size < vma->vm_end - vma->vm_start)
- return -EINVAL;
-
- if (!obj->base.filp)
+ if (!obj)
return -ENODEV;
+ if (obj->base.size < vma->vm_end - vma->vm_start) {
+ ret = -EINVAL;
+ goto out;
+ }
+
+ if (!obj->base.filp) {
+ ret = -ENODEV;
+ goto out;
+ }
+
ret = call_mmap(obj->base.filp, vma);
if (ret)
- return ret;
+ goto out;
fput(vma->vm_file);
vma->vm_file = get_file(obj->base.filp);
- return 0;
+out:
+ i915_gem_object_put(obj);
+ return ret;
}
static int i915_gem_begin_cpu_access(struct dma_buf *dma_buf, enum dma_data_direction direction)
{
- struct drm_i915_gem_object *obj = dma_buf_to_obj(dma_buf);
+ struct drm_i915_gem_object *obj = dmabuf_get_obj(dma_buf->priv);
bool write = (direction == DMA_BIDIRECTIONAL || direction == DMA_TO_DEVICE);
int err;
+ if (!obj)
+ return -ENODEV;
+
err = i915_gem_object_pin_pages(obj);
if (err)
- return err;
+ goto out_put;
err = i915_gem_object_lock_interruptible(obj);
if (err)
- goto out;
+ goto out_unpin;
err = i915_gem_object_set_to_cpu_domain(obj, write);
i915_gem_object_unlock(obj);
-out:
+out_unpin:
i915_gem_object_unpin_pages(obj);
+out_put:
+ i915_gem_object_put(obj);
return err;
}
static int i915_gem_end_cpu_access(struct dma_buf *dma_buf, enum dma_data_direction direction)
{
- struct drm_i915_gem_object *obj = dma_buf_to_obj(dma_buf);
+ struct drm_i915_gem_object *obj = dmabuf_get_obj(dma_buf->priv);
int err;
+ if (!obj)
+ return -ENODEV;
+
err = i915_gem_object_pin_pages(obj);
if (err)
- return err;
+ goto out_put;
err = i915_gem_object_lock_interruptible(obj);
if (err)
- goto out;
+ goto out_unpin;
err = i915_gem_object_set_to_gtt_domain(obj, false);
i915_gem_object_unlock(obj);
-out:
+out_unpin:
i915_gem_object_unpin_pages(obj);
+out_put:
+ i915_gem_object_put(obj);
return err;
}
static const struct dma_buf_ops i915_dmabuf_ops = {
.map_dma_buf = i915_gem_map_dma_buf,
.unmap_dma_buf = i915_gem_unmap_dma_buf,
- .release = drm_gem_dmabuf_release,
+ .release = i915_gem_dmabuf_release,
.mmap = i915_gem_dmabuf_mmap,
.vmap = i915_gem_dmabuf_vmap,
.vunmap = i915_gem_dmabuf_vunmap,
@@ -175,7 +259,25 @@ static const struct dma_buf_ops i915_dmabuf_ops = {
struct dma_buf *i915_gem_prime_export(struct drm_gem_object *gem_obj, int flags)
{
struct drm_i915_gem_object *obj = to_intel_bo(gem_obj);
+ struct drm_i915_private *i915 = to_i915(obj->base.dev);
+ struct i915_dmabuf_priv *dmabuf_priv;
DEFINE_DMA_BUF_EXPORT_INFO(exp_info);
+ struct dma_buf *dma_buf;
+
+ if (obj->ops->dmabuf_export) {
+ int ret = obj->ops->dmabuf_export(obj);
+ if (ret)
+ return ERR_PTR(ret);
+ }
+
+ dmabuf_priv = kzalloc(sizeof(*dmabuf_priv), GFP_KERNEL);
+ if (!dmabuf_priv)
+ return ERR_PTR(-ENOMEM);
+
+ dmabuf_priv->i915 = i915;
+ dmabuf_priv->obj = obj;
+ spin_lock_init(&dmabuf_priv->lock);
+ INIT_LIST_HEAD(&dmabuf_priv->link);
exp_info.ops = &i915_dmabuf_ops;
exp_info.size = gem_obj->size;
@@ -183,13 +285,19 @@ struct dma_buf *i915_gem_prime_export(struct drm_gem_object *gem_obj, int flags)
exp_info.priv = gem_obj;
exp_info.resv = obj->base.resv;
- if (obj->ops->dmabuf_export) {
- int ret = obj->ops->dmabuf_export(obj);
- if (ret)
- return ERR_PTR(ret);
+ dma_buf = drm_gem_dmabuf_export(gem_obj->dev, &exp_info);
+ if (IS_ERR(dma_buf)) {
+ kfree(dmabuf_priv);
+ return dma_buf;
}
- return drm_gem_dmabuf_export(gem_obj->dev, &exp_info);
+ dma_buf->priv = dmabuf_priv;
+
+ mutex_lock(&i915->gem.dmabuf.lock);
+ list_add(&dmabuf_priv->link, &i915->gem.dmabuf.list);
+ mutex_unlock(&i915->gem.dmabuf.lock);
+
+ return dma_buf;
}
static int i915_gem_object_get_pages_dmabuf(struct drm_i915_gem_object *obj)
@@ -232,15 +340,21 @@ struct drm_gem_object *i915_gem_prime_import(struct drm_device *dev,
/* is this one of own objects? */
if (dma_buf->ops == &i915_dmabuf_ops) {
- obj = dma_buf_to_obj(dma_buf);
+ obj = dmabuf_get_obj(dma_buf->priv);
+ if (!obj)
+ return ERR_PTR(-ENODEV);
+
/* is it from our device? */
if (obj->base.dev == dev) {
/*
* Importing dmabuf exported from out own gem increases
* refcount on gem itself instead of f_count of dmabuf.
*/
- return &i915_gem_object_get(obj)->base;
+
+ return &obj->base;
}
+
+ i915_gem_object_put(obj);
}
/* need to attach */
@@ -280,6 +394,24 @@ struct drm_gem_object *i915_gem_prime_import(struct drm_device *dev,
return ERR_PTR(ret);
}
+void i915_gem_dmabuf_driver_probe(struct drm_i915_private *i915)
+{
+ INIT_LIST_HEAD(&i915->gem.dmabuf.list);
+ mutex_init(&i915->gem.dmabuf.lock);
+}
+
+void i915_gem_dmabuf_driver_remove(struct drm_i915_private *i915)
+{
+ struct i915_dmabuf_priv *dmabuf_priv, *dpn;
+
+ mutex_lock(&i915->gem.dmabuf.lock);
+ list_for_each_entry_safe(dmabuf_priv, dpn, &i915->gem.dmabuf.list, link)
+ i915_gem_dmabuf_remove(dmabuf_priv);
+ mutex_unlock(&i915->gem.dmabuf.lock);
+
+ drm_WARN_ON(&i915->drm, !list_empty(&i915->gem.dmabuf.list));
+}
+
#if IS_ENABLED(CONFIG_DRM_I915_SELFTEST)
#include "selftests/mock_dmabuf.c"
#include "selftests/i915_gem_dmabuf.c"
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 6e9072ab30a1..cfb0ecc3da2b 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1195,6 +1195,11 @@ struct drm_i915_private {
* share the global mapping.
*/
struct file *mmap_singleton;
+
+ struct {
+ struct list_head list;
+ struct mutex lock;
+ } dmabuf;
} gem;
u8 pch_ssc_use;
@@ -1737,6 +1742,8 @@ void i915_gem_init_early(struct drm_i915_private *dev_priv);
void i915_gem_cleanup_early(struct drm_i915_private *dev_priv);
int i915_gem_freeze(struct drm_i915_private *dev_priv);
int i915_gem_freeze_late(struct drm_i915_private *dev_priv);
+void i915_gem_dmabuf_driver_probe(struct drm_i915_private *dev_priv);
+void i915_gem_dmabuf_driver_remove(struct drm_i915_private *dev_priv);
struct intel_memory_region *i915_gem_shmem_setup(struct drm_i915_private *i915);
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 106f730c95c4..9cfabb8df922 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1122,6 +1122,8 @@ int i915_gem_init(struct drm_i915_private *dev_priv)
if (ret)
return ret;
+ i915_gem_dmabuf_driver_probe(dev_priv);
+
intel_uc_fetch_firmwares(&dev_priv->gt.uc);
intel_wopcm_init(&dev_priv->wopcm);
@@ -1220,6 +1222,8 @@ static void i915_gem_remove_user_resources(struct drm_i915_private *i915)
i915_gem_driver_release__contexts(i915);
drm_WARN_ON(&i915->drm, !list_empty(&i915->gem.contexts.list));
+
+ i915_gem_dmabuf_driver_remove(i915);
}
void i915_gem_driver_remove(struct drm_i915_private *dev_priv)
diff --git a/drivers/gpu/drm/i915/selftests/mock_gem_device.c b/drivers/gpu/drm/i915/selftests/mock_gem_device.c
index 9b105b811f1f..feca95913923 100644
--- a/drivers/gpu/drm/i915/selftests/mock_gem_device.c
+++ b/drivers/gpu/drm/i915/selftests/mock_gem_device.c
@@ -187,6 +187,8 @@ struct drm_i915_private *mock_gem_device(void)
mock_init_contexts(i915);
+ i915_gem_dmabuf_driver_probe(i915);
+
mock_init_ggtt(i915, &i915->ggtt);
i915->gt.vm = i915_vm_get(&i915->ggtt.vm);
--
2.21.1
More information about the Intel-gfx-trybot
mailing list