Sometimes we need to create a struct file to wrap a drm_device, as it the user were to have opened /dev/dri/card0 but to do so anonymously (i.e. for internal use). Provide a utility method to create a struct file with the drm_device->driver.fops, that wrap the drm_device.
Signed-off-by: Chris Wilson chris@chris-wilson.co.uk --- drivers/gpu/drm/drm_file.c | 39 ++++++++++++++++++++++++++++++++++++++ include/drm/drm_file.h | 3 +++ 2 files changed, 42 insertions(+)
diff --git a/drivers/gpu/drm/drm_file.c b/drivers/gpu/drm/drm_file.c index ea34bc991858..cc55c0cd5826 100644 --- a/drivers/gpu/drm/drm_file.c +++ b/drivers/gpu/drm/drm_file.c @@ -31,7 +31,9 @@ * OTHER DEALINGS IN THE SOFTWARE. */
+#include <linux/anon_inodes.h> #include <linux/dma-fence.h> +#include <linux/file.h> #include <linux/module.h> #include <linux/pci.h> #include <linux/poll.h> @@ -754,3 +756,40 @@ void drm_send_event(struct drm_device *dev, struct drm_pending_event *e) spin_unlock_irqrestore(&dev->event_lock, irqflags); } EXPORT_SYMBOL(drm_send_event); + +/** + * anon_drm_getfile - Create a new struct file for the drm device + * @minor: drm minor to wrap (e.g. drm_device->primary) + * @flags: file creation mode (O_RDWR etc) + * + * This create a new struct file that wraps a DRM file context around a + * DRM minor. This mimicks userspace opening e.g. /dev/dri/card0, but without + * invoking userspace. The struct file may be operated on using its f_op + * (the drm_device.driver.fops) to mimick userspace operations, or be supplied + * to userspace facing functions as an internal/anonymous client. + * + * RETURNS: + * Pointer to newly created struct file, ERR_PTR on failure. + */ +struct file *anon_drm_getfile(struct drm_minor *minor, unsigned int flags) +{ + struct drm_device *dev = minor->dev; + struct drm_file *priv; + struct file *file; + + priv = drm_file_alloc(minor); + if (IS_ERR(priv)) + return ERR_CAST(priv); + + file = anon_inode_getfile("drm", dev->driver->fops, priv, flags); + if (IS_ERR(file)) { + drm_file_free(priv); + return file; + } + + drm_dev_get(dev); + priv->filp = file; + + return file; +} +EXPORT_SYMBOL(anon_drm_getfile); diff --git a/include/drm/drm_file.h b/include/drm/drm_file.h index 67af60bb527a..b963535964f7 100644 --- a/include/drm/drm_file.h +++ b/include/drm/drm_file.h @@ -42,6 +42,7 @@ struct dma_fence; struct drm_file; struct drm_device; struct device; +struct file;
/* * FIXME: Not sure we want to have drm_minor here in the end, but to avoid @@ -387,4 +388,6 @@ void drm_event_cancel_free(struct drm_device *dev, void drm_send_event_locked(struct drm_device *dev, struct drm_pending_event *e); void drm_send_event(struct drm_device *dev, struct drm_pending_event *e);
+struct file *anon_drm_getfile(struct drm_minor *minor, unsigned int flags); + #endif /* _DRM_FILE_H_ */
Provide a utility function to create a vma corresponding to an mmap() of our device. And use it to exercise the equivalent of userspace performing a GTT mmap of our objects.
Signed-off-by: Chris Wilson chris@chris-wilson.co.uk Cc: Abdiel Janulgue abdiel.janulgue@linux.intel.com --- drivers/gpu/drm/i915/Makefile | 1 + .../drm/i915/gem/selftests/i915_gem_mman.c | 97 +++++++++++++++++++ drivers/gpu/drm/i915/selftests/igt_mmap.c | 39 ++++++++ drivers/gpu/drm/i915/selftests/igt_mmap.h | 19 ++++ 4 files changed, 156 insertions(+) create mode 100644 drivers/gpu/drm/i915/selftests/igt_mmap.c create mode 100644 drivers/gpu/drm/i915/selftests/igt_mmap.h
diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile index 90dcf09f52cc..e0fd10c0cfb8 100644 --- a/drivers/gpu/drm/i915/Makefile +++ b/drivers/gpu/drm/i915/Makefile @@ -259,6 +259,7 @@ i915-$(CONFIG_DRM_I915_SELFTEST) += \ selftests/i915_selftest.o \ selftests/igt_flush_test.o \ selftests/igt_live_test.o \ + selftests/igt_mmap.o \ selftests/igt_reset.o \ selftests/igt_spinner.o
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 29b2077b73d2..11a99d5d2ff8 100644 --- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c +++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c @@ -12,6 +12,7 @@ #include "i915_selftest.h" #include "selftests/i915_random.h" #include "selftests/igt_flush_test.h" +#include "selftests/igt_mmap.h"
struct tile { unsigned int width; @@ -694,12 +695,108 @@ static int igt_mmap_offset_exhaustion(void *arg) goto out; }
+#define expand32(x) (((x) << 0) | ((x) << 8) | ((x) << 16) | ((x) << 24)) +static int igt_mmap_gtt(void *arg) +{ + struct drm_i915_private *i915 = arg; + struct drm_i915_gem_object *obj; + struct vm_area_struct *area; + unsigned long addr; + void *vaddr; + int err, i; + + obj = i915_gem_object_create_internal(i915, PAGE_SIZE); + if (IS_ERR(obj)) + return PTR_ERR(obj); + + vaddr = i915_gem_object_pin_map(obj, I915_MAP_WB); + if (IS_ERR(vaddr)) { + err = PTR_ERR(vaddr); + goto out; + } + memset(vaddr, POISON_INUSE, PAGE_SIZE); + i915_gem_object_flush_map(obj); + i915_gem_object_unpin_map(obj); + + err = create_mmap_offset(obj); + if (err) + goto out; + + addr = igt_mmap_node(i915, &obj->base.vma_node, + 0, PROT_WRITE, MAP_SHARED); + if (IS_ERR_VALUE(addr)) { + err = addr; + goto out; + } + + pr_info("igt_mmap(obj:gtt) @ %lx\n", addr); + + area = find_vma(current->mm, addr); + if (!area) { + pr_err("Did not create a vm_area_struct for the mmap\n"); + err = -EINVAL; + goto out_unmap; + } + + if (area->vm_private_data != obj) { + pr_err("vm_area_struct did not point back to our object!\n"); + err = -EINVAL; + goto out_unmap; + } + + for (i = 0; i < PAGE_SIZE / sizeof(u32); i++) { + u32 __user *ux = u64_to_user_ptr((u64)(addr + i * sizeof*(ux))); + u32 x; + + if (get_user(x, ux)) { + pr_err("Unable to read from GTT mmap, offset:%zd\n", + i * sizeof(x)); + err = -EFAULT; + break; + } + + if (x != expand32(POISON_INUSE)) { + pr_err("Read incorrect value from GTT mmap, offset:%zd, found:%x, expected:%x\n", + i * sizeof(x), x, expand32(POISON_INUSE)); + err = -EINVAL; + break; + } + + x = expand32(POISON_FREE); + if (put_user(x, ux)) { + pr_err("Unable to write to GTT mmap, offset:%zd\n", + i * sizeof(x)); + err = -EFAULT; + break; + } + } + +out_unmap: + vm_munmap(addr, PAGE_SIZE); + + vaddr = i915_gem_object_pin_map(obj, I915_MAP_FORCE_WC); + if (IS_ERR(vaddr)) { + err = PTR_ERR(vaddr); + goto out; + } + if (err == 0 && memchr_inv(vaddr, POISON_FREE, PAGE_SIZE)) { + pr_err("Write via GGTT mmap did not land in backing store\n"); + err = -EINVAL; + } + i915_gem_object_unpin_map(obj); + +out: + i915_gem_object_put(obj); + return err; +} + int i915_gem_mman_live_selftests(struct drm_i915_private *i915) { static const struct i915_subtest tests[] = { SUBTEST(igt_partial_tiling), SUBTEST(igt_smoke_tiling), SUBTEST(igt_mmap_offset_exhaustion), + SUBTEST(igt_mmap_gtt), };
return i915_subtests(tests, i915); diff --git a/drivers/gpu/drm/i915/selftests/igt_mmap.c b/drivers/gpu/drm/i915/selftests/igt_mmap.c new file mode 100644 index 000000000000..f5bc80e92573 --- /dev/null +++ b/drivers/gpu/drm/i915/selftests/igt_mmap.c @@ -0,0 +1,39 @@ +/* + * SPDX-License-Identifier: MIT + * + * Copyright © 2019 Intel Corporation + */ + +#include <drm/drm_file.h> + +#include "i915_drv.h" +#include "igt_mmap.h" + +unsigned long igt_mmap_node(struct drm_i915_private *i915, + struct drm_vma_offset_node *node, + unsigned long addr, + unsigned long prot, + unsigned long flags) +{ + struct file *file; + int err; + + /* Pretend to open("/dev/dri/card0") */ + file = anon_drm_getfile(i915->drm.primary, O_RDWR); + if (IS_ERR(file)) + return PTR_ERR(file); + + err = drm_vma_node_allow(node, file->private_data); + if (err) { + addr = err; + goto out_file; + } + + addr = vm_mmap(file, addr, drm_vma_node_size(node) << PAGE_SHIFT, + prot, flags, drm_vma_node_offset_addr(node)); + + drm_vma_node_revoke(node, file->private_data); +out_file: + fput(file); + return addr; +} diff --git a/drivers/gpu/drm/i915/selftests/igt_mmap.h b/drivers/gpu/drm/i915/selftests/igt_mmap.h new file mode 100644 index 000000000000..6e716cb59d7e --- /dev/null +++ b/drivers/gpu/drm/i915/selftests/igt_mmap.h @@ -0,0 +1,19 @@ +/* + * SPDX-License-Identifier: MIT + * + * Copyright © 2019 Intel Corporation + */ + +#ifndef IGT_MMAP_H +#define IGT_MMAP_H + +struct drm_i915_private; +struct drm_vma_offset_node; + +unsigned long igt_mmap_node(struct drm_i915_private *i915, + struct drm_vma_offset_node *node, + unsigned long addr, + unsigned long prot, + unsigned long flags); + +#endif /* IGT_MMAP_H */
Currently the drm_prime mmap fallback uses a mock struct file to provide the file pointer into the backend mmap routine. Now that we can create fully fledged anonymous struct file around the drm device, put it to use.
Signed-off-by: Chris Wilson chris@chris-wilson.co.uk --- drivers/gpu/drm/drm_prime.c | 26 ++++++++------------------ 1 file changed, 8 insertions(+), 18 deletions(-)
diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c index 0814211b0f3f..5faa63713ec8 100644 --- a/drivers/gpu/drm/drm_prime.c +++ b/drivers/gpu/drm/drm_prime.c @@ -709,8 +709,7 @@ EXPORT_SYMBOL(drm_gem_dmabuf_vunmap); */ int drm_gem_prime_mmap(struct drm_gem_object *obj, struct vm_area_struct *vma) { - struct drm_file *priv; - struct file *fil; + struct file *file; int ret;
if (obj->funcs && obj->funcs->mmap) { @@ -722,30 +721,21 @@ int drm_gem_prime_mmap(struct drm_gem_object *obj, struct vm_area_struct *vma) return 0; }
- priv = kzalloc(sizeof(*priv), GFP_KERNEL); - fil = kzalloc(sizeof(*fil), GFP_KERNEL); - if (!priv || !fil) { - ret = -ENOMEM; - goto out; - } + file = anon_drm_getfile(obj->dev->primary, O_RDWR); + if (IS_ERR(file)) + return PTR_ERR(file);
- /* Used by drm_gem_mmap() to lookup the GEM object */ - priv->minor = obj->dev->primary; - fil->private_data = priv; - - ret = drm_vma_node_allow(&obj->vma_node, priv); + ret = drm_vma_node_allow(&obj->vma_node, file->private_data); if (ret) goto out;
vma->vm_pgoff += drm_vma_node_start(&obj->vma_node);
- ret = obj->dev->driver->fops->mmap(fil, vma); + ret = file->f_op->mmap(file, vma);
- drm_vma_node_revoke(&obj->vma_node, priv); + drm_vma_node_revoke(&obj->vma_node, file->private_data); out: - kfree(priv); - kfree(fil); - + fput(file); return ret; } EXPORT_SYMBOL(drm_gem_prime_mmap);
On Wed, Nov 06, 2019 at 10:07:16AM +0000, Chris Wilson wrote:
Currently the drm_prime mmap fallback uses a mock struct file to provide the file pointer into the backend mmap routine. Now that we can create fully fledged anonymous struct file around the drm device, put it to use.
Signed-off-by: Chris Wilson chris@chris-wilson.co.uk
drivers/gpu/drm/drm_prime.c | 26 ++++++++------------------ 1 file changed, 8 insertions(+), 18 deletions(-)
diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c index 0814211b0f3f..5faa63713ec8 100644 --- a/drivers/gpu/drm/drm_prime.c +++ b/drivers/gpu/drm/drm_prime.c @@ -709,8 +709,7 @@ EXPORT_SYMBOL(drm_gem_dmabuf_vunmap); */ int drm_gem_prime_mmap(struct drm_gem_object *obj, struct vm_area_struct *vma) {
- struct drm_file *priv;
- struct file *fil;
struct file *file; int ret;
if (obj->funcs && obj->funcs->mmap) {
obj->funcs->mmap is the new way of doing this (and hopefully finally something clean), I'd really like to retire the below hack outright.
Plus I'm not sure why you need an anon inode here? If a driver needs this for unmap_mapping_range or similar I think it'd be better to try and make something work cleanly for obj->funcs->mmap. -Daniel
@@ -722,30 +721,21 @@ int drm_gem_prime_mmap(struct drm_gem_object *obj, struct vm_area_struct *vma) return 0; }
- priv = kzalloc(sizeof(*priv), GFP_KERNEL);
- fil = kzalloc(sizeof(*fil), GFP_KERNEL);
- if (!priv || !fil) {
ret = -ENOMEM;goto out;- }
- file = anon_drm_getfile(obj->dev->primary, O_RDWR);
- if (IS_ERR(file))
return PTR_ERR(file);
- /* Used by drm_gem_mmap() to lookup the GEM object */
- priv->minor = obj->dev->primary;
- fil->private_data = priv;
- ret = drm_vma_node_allow(&obj->vma_node, priv);
ret = drm_vma_node_allow(&obj->vma_node, file->private_data); if (ret) goto out;
vma->vm_pgoff += drm_vma_node_start(&obj->vma_node);
- ret = obj->dev->driver->fops->mmap(fil, vma);
- ret = file->f_op->mmap(file, vma);
- drm_vma_node_revoke(&obj->vma_node, priv);
- drm_vma_node_revoke(&obj->vma_node, file->private_data);
out:
- kfree(priv);
- kfree(fil);
- fput(file); return ret;
} EXPORT_SYMBOL(drm_gem_prime_mmap); -- 2.24.0
dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Quoting Daniel Vetter (2019-11-06 10:21:57)
On Wed, Nov 06, 2019 at 10:07:16AM +0000, Chris Wilson wrote:
Currently the drm_prime mmap fallback uses a mock struct file to provide the file pointer into the backend mmap routine. Now that we can create fully fledged anonymous struct file around the drm device, put it to use.
Signed-off-by: Chris Wilson chris@chris-wilson.co.uk
drivers/gpu/drm/drm_prime.c | 26 ++++++++------------------ 1 file changed, 8 insertions(+), 18 deletions(-)
diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c index 0814211b0f3f..5faa63713ec8 100644 --- a/drivers/gpu/drm/drm_prime.c +++ b/drivers/gpu/drm/drm_prime.c @@ -709,8 +709,7 @@ EXPORT_SYMBOL(drm_gem_dmabuf_vunmap); */ int drm_gem_prime_mmap(struct drm_gem_object *obj, struct vm_area_struct *vma) {
struct drm_file *priv;struct file *fil;
struct file *file; int ret; if (obj->funcs && obj->funcs->mmap) {obj->funcs->mmap is the new way of doing this (and hopefully finally something clean), I'd really like to retire the below hack outright.
Plus I'm not sure why you need an anon inode here? If a driver needs this for unmap_mapping_range or similar I think it'd be better to try and make something work cleanly for obj->funcs->mmap.
It's faking one currently. If the fake is not good enough, you are playing whack-a-mole until you finally do create a fully fledged file.
If you are going to the trouble of having to create a struct file to provide to the fallback routines, might as well avoid stinky code :) -Chris
On Wed, Nov 6, 2019 at 11:45 AM Chris Wilson chris@chris-wilson.co.uk wrote:
Quoting Daniel Vetter (2019-11-06 10:21:57)
On Wed, Nov 06, 2019 at 10:07:16AM +0000, Chris Wilson wrote:
Currently the drm_prime mmap fallback uses a mock struct file to provide the file pointer into the backend mmap routine. Now that we can create fully fledged anonymous struct file around the drm device, put it to use.
Signed-off-by: Chris Wilson chris@chris-wilson.co.uk
drivers/gpu/drm/drm_prime.c | 26 ++++++++------------------ 1 file changed, 8 insertions(+), 18 deletions(-)
diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c index 0814211b0f3f..5faa63713ec8 100644 --- a/drivers/gpu/drm/drm_prime.c +++ b/drivers/gpu/drm/drm_prime.c @@ -709,8 +709,7 @@ EXPORT_SYMBOL(drm_gem_dmabuf_vunmap); */ int drm_gem_prime_mmap(struct drm_gem_object *obj, struct vm_area_struct *vma) {
struct drm_file *priv;struct file *fil;
struct file *file; int ret; if (obj->funcs && obj->funcs->mmap) {obj->funcs->mmap is the new way of doing this (and hopefully finally something clean), I'd really like to retire the below hack outright.
Plus I'm not sure why you need an anon inode here? If a driver needs this for unmap_mapping_range or similar I think it'd be better to try and make something work cleanly for obj->funcs->mmap.
It's faking one currently. If the fake is not good enough, you are playing whack-a-mole until you finally do create a fully fledged file.
If you are going to the trouble of having to create a struct file to provide to the fallback routines, might as well avoid stinky code :)
We're currently not faking the inode at all, we're just using the one that comes with the dma-buf. So distinct from the drm_device file, and hence unmap_mapping_range won't work (or at least doing that on the drm_device inode wont shoot down the ptes for redirected dma-buf mmaps). obj->funcs->mmap has the same issue.
But since all current users of this don't expect unmap_mapping_range to work correctly, it's not an real issue. If that changes then imo we should fix up the obj->funcs->mmap path to have the correct inode, not the deprecated path you're updating here. But since there's no patch 4 in this series to start using this for i915 or someone else, I'm not seeing the point.
Or am I blind? At least slightly confused, -Daniel
Quoting Daniel Vetter (2019-11-06 13:06:26)
On Wed, Nov 6, 2019 at 11:45 AM Chris Wilson chris@chris-wilson.co.uk wrote:
Quoting Daniel Vetter (2019-11-06 10:21:57)
On Wed, Nov 06, 2019 at 10:07:16AM +0000, Chris Wilson wrote:
Currently the drm_prime mmap fallback uses a mock struct file to provide the file pointer into the backend mmap routine. Now that we can create fully fledged anonymous struct file around the drm device, put it to use.
Signed-off-by: Chris Wilson chris@chris-wilson.co.uk
drivers/gpu/drm/drm_prime.c | 26 ++++++++------------------ 1 file changed, 8 insertions(+), 18 deletions(-)
diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c index 0814211b0f3f..5faa63713ec8 100644 --- a/drivers/gpu/drm/drm_prime.c +++ b/drivers/gpu/drm/drm_prime.c @@ -709,8 +709,7 @@ EXPORT_SYMBOL(drm_gem_dmabuf_vunmap); */ int drm_gem_prime_mmap(struct drm_gem_object *obj, struct vm_area_struct *vma) {
struct drm_file *priv;struct file *fil;
struct file *file; int ret; if (obj->funcs && obj->funcs->mmap) {obj->funcs->mmap is the new way of doing this (and hopefully finally something clean), I'd really like to retire the below hack outright.
Plus I'm not sure why you need an anon inode here? If a driver needs this for unmap_mapping_range or similar I think it'd be better to try and make something work cleanly for obj->funcs->mmap.
It's faking one currently. If the fake is not good enough, you are playing whack-a-mole until you finally do create a fully fledged file.
If you are going to the trouble of having to create a struct file to provide to the fallback routines, might as well avoid stinky code :)
We're currently not faking the inode at all, we're just using the one that comes with the dma-buf. So distinct from the drm_device file, and hence unmap_mapping_range won't work (or at least doing that on the drm_device inode wont shoot down the ptes for redirected dma-buf mmaps). obj->funcs->mmap has the same issue.
But since all current users of this don't expect unmap_mapping_range to work correctly, it's not an real issue. If that changes then imo we should fix up the obj->funcs->mmap path to have the correct inode, not the deprecated path you're updating here. But since there's no patch 4 in this series to start using this for i915 or someone else, I'm not seeing the point.
There's a bug in anon_drm_inode() in that it requires an extra:
+ /* Everyone shares a single global address space */ + file->f_mapping = dev->anon_inode->i_mapping;
I'm up to 5 patches now, but only i915/selftests & this here fallback as direct users. -Chris
On Wed, Nov 06, 2019 at 10:07:14AM +0000, Chris Wilson wrote:
Sometimes we need to create a struct file to wrap a drm_device, as it the user were to have opened /dev/dri/card0 but to do so anonymously (i.e. for internal use). Provide a utility method to create a struct file with the drm_device->driver.fops, that wrap the drm_device.
Signed-off-by: Chris Wilson chris@chris-wilson.co.uk
For proper internal access we already have drm_client_open, so I think this has limited (but good use) in selftests only. So EXPORT_SYMBOL_FOR_TESTS_ONLY plus maybe a clearer name for the intended use like drm_file_mock_open?
Aside from this I like. -Daniel
drivers/gpu/drm/drm_file.c | 39 ++++++++++++++++++++++++++++++++++++++ include/drm/drm_file.h | 3 +++ 2 files changed, 42 insertions(+)
diff --git a/drivers/gpu/drm/drm_file.c b/drivers/gpu/drm/drm_file.c index ea34bc991858..cc55c0cd5826 100644 --- a/drivers/gpu/drm/drm_file.c +++ b/drivers/gpu/drm/drm_file.c @@ -31,7 +31,9 @@
- OTHER DEALINGS IN THE SOFTWARE.
*/
+#include <linux/anon_inodes.h> #include <linux/dma-fence.h> +#include <linux/file.h> #include <linux/module.h> #include <linux/pci.h> #include <linux/poll.h> @@ -754,3 +756,40 @@ void drm_send_event(struct drm_device *dev, struct drm_pending_event *e) spin_unlock_irqrestore(&dev->event_lock, irqflags); } EXPORT_SYMBOL(drm_send_event);
+/**
- anon_drm_getfile - Create a new struct file for the drm device
- @minor: drm minor to wrap (e.g. drm_device->primary)
- @flags: file creation mode (O_RDWR etc)
- This create a new struct file that wraps a DRM file context around a
- DRM minor. This mimicks userspace opening e.g. /dev/dri/card0, but without
- invoking userspace. The struct file may be operated on using its f_op
- (the drm_device.driver.fops) to mimick userspace operations, or be supplied
- to userspace facing functions as an internal/anonymous client.
- RETURNS:
- Pointer to newly created struct file, ERR_PTR on failure.
- */
+struct file *anon_drm_getfile(struct drm_minor *minor, unsigned int flags) +{
- struct drm_device *dev = minor->dev;
- struct drm_file *priv;
- struct file *file;
- priv = drm_file_alloc(minor);
- if (IS_ERR(priv))
return ERR_CAST(priv);- file = anon_inode_getfile("drm", dev->driver->fops, priv, flags);
- if (IS_ERR(file)) {
drm_file_free(priv);return file;- }
- drm_dev_get(dev);
- priv->filp = file;
- return file;
+} +EXPORT_SYMBOL(anon_drm_getfile); diff --git a/include/drm/drm_file.h b/include/drm/drm_file.h index 67af60bb527a..b963535964f7 100644 --- a/include/drm/drm_file.h +++ b/include/drm/drm_file.h @@ -42,6 +42,7 @@ struct dma_fence; struct drm_file; struct drm_device; struct device; +struct file;
/*
- FIXME: Not sure we want to have drm_minor here in the end, but to avoid
@@ -387,4 +388,6 @@ void drm_event_cancel_free(struct drm_device *dev, void drm_send_event_locked(struct drm_device *dev, struct drm_pending_event *e); void drm_send_event(struct drm_device *dev, struct drm_pending_event *e);
+struct file *anon_drm_getfile(struct drm_minor *minor, unsigned int flags);
#endif /* _DRM_FILE_H_ */
2.24.0
Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Quoting Daniel Vetter (2019-11-06 10:19:50)
On Wed, Nov 06, 2019 at 10:07:14AM +0000, Chris Wilson wrote:
Sometimes we need to create a struct file to wrap a drm_device, as it the user were to have opened /dev/dri/card0 but to do so anonymously (i.e. for internal use). Provide a utility method to create a struct file with the drm_device->driver.fops, that wrap the drm_device.
Signed-off-by: Chris Wilson chris@chris-wilson.co.uk
For proper internal access we already have drm_client_open, so I think this has limited (but good use) in selftests only. So EXPORT_SYMBOL_FOR_TESTS_ONLY plus maybe a clearer name for the intended use like drm_file_mock_open?
I found the example in drm_gem_prime_mmap() that was doing the same trick, and the trick of being able to instantiate new struct file and install a fd whenever seems like it will come in handy... Just lacking the third user at the moment to claim generality. -Chris
Quoting Chris Wilson (2019-11-06 10:26:48)
Quoting Daniel Vetter (2019-11-06 10:19:50)
On Wed, Nov 06, 2019 at 10:07:14AM +0000, Chris Wilson wrote:
Sometimes we need to create a struct file to wrap a drm_device, as it the user were to have opened /dev/dri/card0 but to do so anonymously (i.e. for internal use). Provide a utility method to create a struct file with the drm_device->driver.fops, that wrap the drm_device.
Signed-off-by: Chris Wilson chris@chris-wilson.co.uk
For proper internal access we already have drm_client_open, so I think this has limited (but good use) in selftests only. So EXPORT_SYMBOL_FOR_TESTS_ONLY plus maybe a clearer name for the intended use like drm_file_mock_open?
I found the example in drm_gem_prime_mmap() that was doing the same trick, and the trick of being able to instantiate new struct file and install a fd whenever seems like it will come in handy... Just lacking the third user at the moment to claim generality.
The closest example I found in the spirit of creating a new drm_device struct file and installing it is drm_mode_create_lease_ioctl() that uses file_clone_open() for this purpose. The argument there would be whether cloning the (file->f_path, file->f_flags, file->f_cred) is appropriate versus an anonymous inode. I think cloning the credentials seems correct for leasing. -Chris
On Wed, Nov 6, 2019 at 11:43 AM Chris Wilson chris@chris-wilson.co.uk wrote:
Quoting Chris Wilson (2019-11-06 10:26:48)
Quoting Daniel Vetter (2019-11-06 10:19:50)
On Wed, Nov 06, 2019 at 10:07:14AM +0000, Chris Wilson wrote:
Sometimes we need to create a struct file to wrap a drm_device, as it the user were to have opened /dev/dri/card0 but to do so anonymously (i.e. for internal use). Provide a utility method to create a struct file with the drm_device->driver.fops, that wrap the drm_device.
Signed-off-by: Chris Wilson chris@chris-wilson.co.uk
For proper internal access we already have drm_client_open, so I think this has limited (but good use) in selftests only. So EXPORT_SYMBOL_FOR_TESTS_ONLY plus maybe a clearer name for the intended use like drm_file_mock_open?
I found the example in drm_gem_prime_mmap() that was doing the same trick, and the trick of being able to instantiate new struct file and install a fd whenever seems like it will come in handy... Just lacking the third user at the moment to claim generality.
The closest example I found in the spirit of creating a new drm_device struct file and installing it is drm_mode_create_lease_ioctl() that uses file_clone_open() for this purpose. The argument there would be whether cloning the (file->f_path, file->f_flags, file->f_cred) is appropriate versus an anonymous inode. I think cloning the credentials seems correct for leasing.
Hm ... I think we want the clone for this one here too. Otherwise we get the wrong inode, and then unmap_mapping_range wont work correctly, and we cant use this for selftests. That's the only case where I think we do actually need the file/inode to be functional. For anything else the drm_client internal api gets away without the file/inode stuff. -Daniel
dri-devel@lists.freedesktop.org
-
Chris Wilson -
Daniel Vetter