Hi
This is v2 of the unified VMA offset manager series. The first draft is available at LWN [1]. This series replaces the VMA offset managers in GEM and TTM with a unified implementation.
The first 4 patches contain the new VMA offset manager and are the only patches that I propose for mainline inclusion. Patches 5-8 clean up drm_mm and are informational-only. Daniel has similar patches in i915-next and I will rebase them once it is merged. Ignore them if you're not interested. Patches 9-19 implement mmap access control. Comments are welcome! They are intended for mainline inclusion, too, but probably require some more review rounds. I'd really appreciate if driver authors could comment on the implementation. Patch 20 shows the main motivation behind this whole series: render nodes. It is marked RFC and I will resend once the VMA manager is merged upstream. Feel free to test it.
One more note regarding DRM-Master: Render-clients are independent of DRM-Master (see the DocBook documentation). It really doesn't make sense to create/bind a DRM-Master object to render-clients. However, a lot of core DRM code depends on ->master != NULL. Drivers need to take care not to call into those core modules if they implement DRIVER_RENDER. I plan on removing multiple-master-support in the next series. So there will be only one global master and each open-file is bound to it. This will make it very easy to phase out "drm_master". The planned "modeset" nodes provide a nice replacement. If anyone knows a **currently used** use-case for multiple-masters, please tell me. I couldn't find one that _actually works_. (SetMaster+DropMaster will obviously stay functional even with drm_master removed).
(series available at: https://github.com/dvdhrm/linux/tree/rnodes)
Comments welcome! Cheers David
Changes in v2: - Drop drm_mm_init() fix (already applied in drm-next) - Drop drm_mm_node_linked() in favor of drm_mm_node_allocated() - Adjust DocBook integration - Rebased on drm-next (and compile tested on ARM) - Removed unnecessary likely/unlikely marks (except for rbtree paths) - small fixes (see commit messages for more)
New in v2: - Cleanup patches for drm_mm. Note that these are marked RFC as Daniel has similar patches pending in i915-next. But the patches show that most of drm_mm is obsolete and can be removed. I will rebase them once i915-next is merged into drm-next and resend. - Access Management API: Following Dave's proposal, this series implements mmap-offset access managamenet via a "file_priv" lookup-tree for each node. - Render-Nodes: Following krh's proposal, I revived render nodes which no longer have the mmap security problem. Driver-implementations still missing, but you can find working examples for i915 and nouveau on dri-devel or [2].
Still To Do: - See whether _DRM_GEM can be dropped. I haven't checked legacy user-space so far. - Patch #16 (vmwgfx) has a TODO marker. I couldn't figure out whether the proposed replacement does the same as the old code (with all the side-effects). Would be nice if Thomas could comment on that (CC'ed). - Patch #18 (gem mmap) adds filp to all VMAs during open and removes them during close. This is not necessary for TTM+GEM drivers which don't use gem_mmap(). However, it works for now and I wanted to avoid 20 more patches fixing each GEM driver. I will provide that in the final series.
[1] Unified VMA Offset Manager v1: https://lwn.net/Articles/557265/ [2] Render-node driver patches: https://gitorious.org/linux-nouveau-pm/linux-nouveau-pm/commits/render_nodes
David Herrmann (20): drm: add unified vma offset manager drm/gem: convert to new unified vma manager drm/ttm: convert to unified vma offset manager drm/vma: provide drm_vma_node_unmap() helper drm/mm: add "best_match" to drm_mm_insert_node() drm/ttm: replace drm_mm_pre_get() by direct alloc drm/i915: pre-alloc instead of drm_mm search/get_block drm/mm: remove unused API drm/vma: add access management helpers drm/ast: implement mmap access managament drm/cirrus: implement mmap access managament drm/mgag200: implement mmap access managament drm/nouveau: implement mmap access managament drm/radeon: implement mmap access managament drm/qxl: implement mmap access managament drm/vmwgfx: implement mmap access managament drm/ttm: prevent mmap access to unauthorized users drm/gem: implement mmap access management drm: fix minor number range calculation drm: implement render nodes
Documentation/DocBook/drm.tmpl | 77 ++++++ drivers/gpu/drm/Makefile | 2 +- drivers/gpu/drm/ast/ast_drv.c | 2 + drivers/gpu/drm/ast/ast_drv.h | 4 + drivers/gpu/drm/ast/ast_main.c | 17 +- drivers/gpu/drm/cirrus/cirrus_drv.h | 4 + drivers/gpu/drm/cirrus/cirrus_main.c | 17 +- drivers/gpu/drm/drm_drv.c | 15 +- drivers/gpu/drm/drm_fops.c | 14 +- drivers/gpu/drm/drm_gem.c | 111 +++----- drivers/gpu/drm/drm_gem_cma_helper.c | 9 +- drivers/gpu/drm/drm_mm.c | 139 ++-------- drivers/gpu/drm/drm_pci.c | 9 + drivers/gpu/drm/drm_platform.c | 9 + drivers/gpu/drm/drm_stub.c | 17 +- drivers/gpu/drm/drm_usb.c | 9 + drivers/gpu/drm/drm_vma_manager.c | 408 +++++++++++++++++++++++++++++ drivers/gpu/drm/exynos/exynos_drm_gem.c | 7 +- drivers/gpu/drm/gma500/gem.c | 8 +- drivers/gpu/drm/i915/i915_gem.c | 25 +- drivers/gpu/drm/i915/i915_gem_stolen.c | 72 +++-- drivers/gpu/drm/mgag200/mgag200_drv.c | 2 + drivers/gpu/drm/mgag200/mgag200_drv.h | 4 + drivers/gpu/drm/mgag200/mgag200_main.c | 17 +- drivers/gpu/drm/nouveau/nouveau_display.c | 2 +- drivers/gpu/drm/nouveau/nouveau_gem.c | 21 +- drivers/gpu/drm/omapdrm/omap_gem.c | 11 +- drivers/gpu/drm/omapdrm/omap_gem_helpers.c | 49 +--- drivers/gpu/drm/qxl/qxl_gem.c | 7 +- drivers/gpu/drm/qxl/qxl_object.h | 2 +- drivers/gpu/drm/qxl/qxl_release.c | 2 +- drivers/gpu/drm/radeon/radeon_gem.c | 7 + drivers/gpu/drm/radeon/radeon_object.h | 5 +- drivers/gpu/drm/sis/sis_mm.c | 4 +- drivers/gpu/drm/ttm/ttm_bo.c | 79 +----- drivers/gpu/drm/ttm/ttm_bo_manager.c | 40 ++- drivers/gpu/drm/ttm/ttm_bo_util.c | 3 +- drivers/gpu/drm/ttm/ttm_bo_vm.c | 82 +++--- drivers/gpu/drm/udl/udl_gem.c | 6 +- drivers/gpu/drm/via/via_mm.c | 4 +- drivers/gpu/drm/vmwgfx/vmwgfx_resource.c | 33 ++- include/drm/drmP.h | 15 +- include/drm/drm_mm.h | 110 ++------ include/drm/drm_vma_manager.h | 170 ++++++++++++ include/drm/ttm/ttm_bo_api.h | 15 +- include/drm/ttm/ttm_bo_driver.h | 7 +- include/uapi/drm/drm.h | 2 +- 47 files changed, 1074 insertions(+), 600 deletions(-) create mode 100644 drivers/gpu/drm/drm_vma_manager.c create mode 100644 include/drm/drm_vma_manager.h
If we want to map GPU memory into user-space, we need to linearize the addresses to not confuse mm-core. Currently, GEM and TTM both implement their own offset-managers to assign a pgoff to each object for user-space CPU access. GEM uses a hash-table, TTM uses an rbtree.
This patch provides a unified implementation that can be used to replace both. TTM allows partial mmaps with a given offset, so we cannot use hashtables as the start address may not be known at mmap time. Hence, we use the rbtree-implementation of TTM.
We could easily update drm_mm to use an rbtree instead of a linked list for it's object list and thus drop the rbtree from the vma-manager. However, this would slow down drm_mm object allocation for all other use-cases (rbtree insertion) and add another 4-8 bytes to each mm node. Hence, use the separate tree but allow for later migration.
This is a rewrite of the 2012-proposal by David Airlie airlied@linux.ie
v2: - fix Docbook integration - drop drm_mm_node_linked() and use drm_mm_node_allocated() - remove unjustified likely/unlikely usage (but keep for rbtree paths) - remove BUG_ON() as drm_mm already does that - clarify page-based vs. byte-based addresses - use drm_vma_node_reset() for initialization, too
Signed-off-by: David Herrmann dh.herrmann@gmail.com --- Documentation/DocBook/drm.tmpl | 6 + drivers/gpu/drm/Makefile | 2 +- drivers/gpu/drm/drm_vma_manager.c | 260 ++++++++++++++++++++++++++++++++++++++ include/drm/drm_vma_manager.h | 139 ++++++++++++++++++++ 4 files changed, 406 insertions(+), 1 deletion(-) create mode 100644 drivers/gpu/drm/drm_vma_manager.c create mode 100644 include/drm/drm_vma_manager.h
diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl index 4d54ac8..b02732d 100644 --- a/Documentation/DocBook/drm.tmpl +++ b/Documentation/DocBook/drm.tmpl @@ -2212,6 +2212,12 @@ void intel_crt_init(struct drm_device *dev) !Iinclude/drm/drm_rect.h !Edrivers/gpu/drm/drm_rect.c </sect2> + <sect2> + <title>VMA Offset Manager</title> +!Pdrivers/gpu/drm/drm_vma_manager.c vma offset manager +!Edrivers/gpu/drm/drm_vma_manager.c +!Iinclude/drm/drm_vma_manager.h + </sect2> </sect1>
<!-- Internals: kms properties --> diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile index 801bcaf..d943b94 100644 --- a/drivers/gpu/drm/Makefile +++ b/drivers/gpu/drm/Makefile @@ -13,7 +13,7 @@ drm-y := drm_auth.o drm_buffer.o drm_bufs.o drm_cache.o \ drm_crtc.o drm_modes.o drm_edid.o \ drm_info.o drm_debugfs.o drm_encoder_slave.o \ drm_trace_points.o drm_global.o drm_prime.o \ - drm_rect.o + drm_rect.o drm_vma_manager.o
drm-$(CONFIG_COMPAT) += drm_ioc32.o drm-$(CONFIG_DRM_GEM_CMA_HELPER) += drm_gem_cma_helper.o diff --git a/drivers/gpu/drm/drm_vma_manager.c b/drivers/gpu/drm/drm_vma_manager.c new file mode 100644 index 0000000..4b5f415 --- /dev/null +++ b/drivers/gpu/drm/drm_vma_manager.c @@ -0,0 +1,260 @@ +/* + * Copyright (c) 2006-2009 VMware, Inc., Palo Alto, CA., USA + * Copyright (c) 2012 David Airlie airlied@linux.ie + * Copyright (c) 2013 David Herrmann dh.herrmann@gmail.com + * + * Permission is hereby granted, free of charge, to any person obtaining a + * copy of this software and associated documentation files (the "Software"), + * to deal in the Software without restriction, including without limitation + * the rights to use, copy, modify, merge, publish, distribute, sublicense, + * and/or sell copies of the Software, and to permit persons to whom the + * Software is furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR + * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, + * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR + * OTHER DEALINGS IN THE SOFTWARE. + */ + +#include <drm/drmP.h> +#include <drm/drm_mm.h> +#include <drm/drm_vma_manager.h> +#include <linux/mm.h> +#include <linux/module.h> +#include <linux/rbtree.h> +#include <linux/slab.h> +#include <linux/spinlock.h> +#include <linux/types.h> + +/** + * DOC: vma offset manager + * + * The vma-manager is responsible to map arbitrary driver-dependent memory + * regions into the linear user address-space. It provides offsets to the + * caller which can then be used on the address_space of the drm-device. It + * takes care to not overlap regions, size them appropriately and to not + * confuse mm-core by inconsistent fake vm_pgoff fields. + * Drivers shouldn't use this for object placement in VMEM. This manager should + * only be used to manage mappings into linear user-space VMs. + * + * We use drm_mm as backend to manage object allocations. But it is highly + * optimized for alloc/free calls, not lookups. Hence, we use an rb-tree to + * speed up offset lookups. + * + * You must not use multiple offset managers on a single address_space. + * Otherwise, mm-core will be unable to tear down memory mappings as the VM will + * no longer be linear. Please use VM_NONLINEAR in that case and implement your + * own offset managers. + * + * This offset manager works on page-based addresses. That is, every argument + * and return code (with the exception of drm_vma_node_offset_addr()) is given + * in number of pages, not number of bytes. That means, object sizes and offsets + * must always be page-aligned (as usual). + * If you want to get a valid byte-based user-space address for a given offset, + * please see drm_vma_node_offset_addr(). + */ + +/** + * drm_vma_offset_manager_init - Initialize new offset-manager + * @mgr: Manager object + * @page_offset: Offset of available memory area (page-based) + * @size: Size of available memory area (page-based) + * + * Initialize a new offset-manager. The offset and area size available for the + * manager are given as @page_offset and @size. Both are interpreted as + * page-numbers, not bytes. + * + * Adding/removing nodes from the manager is locked internally and protected + * against concurrent access. However, node allocation and destruction is left + * for the caller. While calling into the vma-manager, a given node must + * always be guaranteed to be referenced. + */ +void drm_vma_offset_manager_init(struct drm_vma_offset_manager *mgr, + unsigned long page_offset, unsigned long size) +{ + rwlock_init(&mgr->vm_lock); + mgr->vm_addr_space_rb = RB_ROOT; + drm_mm_init(&mgr->vm_addr_space_mm, page_offset, size); +} +EXPORT_SYMBOL(drm_vma_offset_manager_init); + +/** + * drm_vma_offset_manager_destroy() - Destroy offset manager + * @mgr: Manager object + * + * Destroy an object manager which was previously created via + * drm_vma_offset_manager_init(). The caller must remove all allocated nodes + * before destroying the manager. Otherwise, drm_mm will refuse to free the + * requested resources. + * + * The manager must not be accessed after this function is called. + */ +void drm_vma_offset_manager_destroy(struct drm_vma_offset_manager *mgr) +{ + /* take the lock to protect against buggy drivers */ + write_lock(&mgr->vm_lock); + drm_mm_takedown(&mgr->vm_addr_space_mm); + write_unlock(&mgr->vm_lock); +} +EXPORT_SYMBOL(drm_vma_offset_manager_destroy); + +/** + * drm_vma_offset_lookup() - Find node in offset space + * @mgr: Manager object + * @start: Start address for object (page-based) + * @pages: Size of object (page-based) + * + * Find a node given a start address and object size. This returns the _best_ + * match for the given node. That is, @start may point somewhere into a valid + * region and the given node will be returned, as long as the node spans the + * whole requested area (given the size in number of pages as @pages). + * + * RETURNS: + * Returns NULL if no suitable node can be found. Otherwise, the best match + * is returned. It's the caller's responsibility to make sure the node doesn't + * get destroyed before the caller can access it. + */ +struct drm_vma_offset_node *drm_vma_offset_lookup(struct drm_vma_offset_manager *mgr, + unsigned long start, + unsigned long pages) +{ + struct drm_vma_offset_node *node, *best; + struct rb_node *iter; + unsigned long offset; + + read_lock(&mgr->vm_lock); + + iter = mgr->vm_addr_space_rb.rb_node; + best = NULL; + + while (likely(iter)) { + node = rb_entry(iter, struct drm_vma_offset_node, vm_rb); + offset = node->vm_node.start; + if (start >= offset) { + iter = iter->rb_right; + best = node; + if (start == offset) + break; + } else { + iter = iter->rb_left; + } + } + + /* verify that the node spans the requested area */ + if (best) { + offset = best->vm_node.start + best->vm_pages; + if (offset > start + pages) + best = NULL; + } + + read_unlock(&mgr->vm_lock); + + return best; +} +EXPORT_SYMBOL(drm_vma_offset_lookup); + +/* internal helper to link @node into the rb-tree */ +static void _drm_vma_offset_add_rb(struct drm_vma_offset_manager *mgr, + struct drm_vma_offset_node *node) +{ + struct rb_node **iter = &mgr->vm_addr_space_rb.rb_node; + struct rb_node *parent = NULL; + struct drm_vma_offset_node *iter_node; + + while (likely(*iter)) { + parent = *iter; + iter_node = rb_entry(*iter, struct drm_vma_offset_node, vm_rb); + + if (node->vm_node.start < iter_node->vm_node.start) + iter = &(*iter)->rb_left; + else if (node->vm_node.start > iter_node->vm_node.start) + iter = &(*iter)->rb_right; + else + BUG(); + } + + rb_link_node(&node->vm_rb, parent, iter); + rb_insert_color(&node->vm_rb, &mgr->vm_addr_space_rb); +} + +/** + * drm_vma_offset_add() - Add offset node to manager + * @mgr: Manager object + * @node: Node to be added + * @pages: Allocation size visible to user-space (in number of pages) + * + * Add a node to the offset-manager. If the node was already added, this does + * nothing and return 0. @pages is the size of the object given in number of + * pages. + * After this call succeeds, you can access the offset of the node until it + * is removed again. + * + * If this call fails, it is safe to retry the operation or call + * drm_vma_offset_remove(), anyway. However, no cleanup is required in that + * case. + * + * @pages is not required to be the same size as the underlying memory object + * that you want to map. It only limits the size that user-space can map into + * their address space. + * + * RETURNS: + * 0 on success, negative error code on failure. + */ +int drm_vma_offset_add(struct drm_vma_offset_manager *mgr, + struct drm_vma_offset_node *node, unsigned long pages) +{ + int ret; + + write_lock(&mgr->vm_lock); + + if (drm_mm_node_allocated(&node->vm_node)) { + ret = 0; + goto out_unlock; + } + + ret = drm_mm_insert_node_generic(&mgr->vm_addr_space_mm, + &node->vm_node, pages, 0, 0); + if (ret) + goto out_unlock; + + node->vm_pages = pages; + _drm_vma_offset_add_rb(mgr, node); + goto out_unlock; + +out_unlock: + write_unlock(&mgr->vm_lock); + return ret; +} +EXPORT_SYMBOL(drm_vma_offset_add); + +/** + * drm_vma_offset_remove() - Remove offset node from manager + * @mgr: Manager object + * @node: Node to be removed + * + * Remove a node from the offset manager. If the node wasn't added before, this + * does nothing. After this call returns, the offset of the node must be not + * accessed, anymore. + * It is safe to add the node via drm_vma_offset_add() again. + */ +void drm_vma_offset_remove(struct drm_vma_offset_manager *mgr, + struct drm_vma_offset_node *node) +{ + write_lock(&mgr->vm_lock); + + if (drm_mm_node_allocated(&node->vm_node)) { + rb_erase(&node->vm_rb, &mgr->vm_addr_space_rb); + drm_mm_remove_node(&node->vm_node); + memset(&node->vm_node, 0, sizeof(node->vm_node)); + node->vm_pages = 0; + } + + write_unlock(&mgr->vm_lock); +} +EXPORT_SYMBOL(drm_vma_offset_remove); diff --git a/include/drm/drm_vma_manager.h b/include/drm/drm_vma_manager.h new file mode 100644 index 0000000..b519d7f --- /dev/null +++ b/include/drm/drm_vma_manager.h @@ -0,0 +1,139 @@ +#ifndef __DRM_VMA_MANAGER_H__ +#define __DRM_VMA_MANAGER_H__ + +/* + * Copyright (c) 2013 David Herrmann dh.herrmann@gmail.com + * + * Permission is hereby granted, free of charge, to any person obtaining a + * copy of this software and associated documentation files (the "Software"), + * to deal in the Software without restriction, including without limitation + * the rights to use, copy, modify, merge, publish, distribute, sublicense, + * and/or sell copies of the Software, and to permit persons to whom the + * Software is furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR + * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, + * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR + * OTHER DEALINGS IN THE SOFTWARE. + */ + +#include <drm/drmP.h> +#include <drm/drm_mm.h> +#include <linux/module.h> +#include <linux/rbtree.h> +#include <linux/spinlock.h> +#include <linux/types.h> + +struct drm_vma_offset_node { + struct drm_mm_node vm_node; + struct rb_node vm_rb; + unsigned long vm_pages; +}; + +struct drm_vma_offset_manager { + rwlock_t vm_lock; + struct rb_root vm_addr_space_rb; + struct drm_mm vm_addr_space_mm; +}; + +void drm_vma_offset_manager_init(struct drm_vma_offset_manager *mgr, + unsigned long page_offset, unsigned long size); +void drm_vma_offset_manager_destroy(struct drm_vma_offset_manager *mgr); + +struct drm_vma_offset_node *drm_vma_offset_lookup(struct drm_vma_offset_manager *mgr, + unsigned long start, + unsigned long pages); +int drm_vma_offset_add(struct drm_vma_offset_manager *mgr, + struct drm_vma_offset_node *node, unsigned long pages); +void drm_vma_offset_remove(struct drm_vma_offset_manager *mgr, + struct drm_vma_offset_node *node); + +/** + * drm_vma_offset_exact_lookup() - Look up node by exact address + * @mgr: Manager object + * @start: Start address (page-based, not byte-based) + * + * Same as drm_vma_offset_lookup() but does not allow any offset into the node. + * It only returns the exact object with the given start address. + * + * RETURNS: + * Node at exact start address @start. + */ +static inline struct drm_vma_offset_node * +drm_vma_offset_exact_lookup(struct drm_vma_offset_manager *mgr, + unsigned long start) +{ + return drm_vma_offset_lookup(mgr, start, 1); +} + +/** + * drm_vma_node_reset() - Initialize or reset node object + * @node: Node to initialize or reset + * + * Reset a node to its initial state. You must call this before using @node with + * any vma offset manager. + * + * This must not be called on an already allocated node, or you will leak + * memory. + */ +static inline void drm_vma_node_reset(struct drm_vma_offset_node *node) +{ + memset(node, 0, sizeof(*node)); +} + +/** + * drm_vma_node_start() - Return start address for page-based addressing + * @node: Node to inspect + * + * Return the start address of the given node. This can be used as offset into + * the linear VM space that is provided by the VMA offset manager. Note that + * this can only be used for page-based addressing. If you need a proper offset + * for user-space mappings, you must apply "<< PAGE_SHIFT" or use the + * drm_vma_node_offset_addr() helper instead. + * + * RETURNS: + * Start address of @node for page-based addressing. 0 if the node does not + * have an offset allocated. + */ +static inline unsigned long drm_vma_node_start(struct drm_vma_offset_node *node) +{ + return node->vm_node.start; +} + +/** + * drm_vma_node_has_offset() - Check whether node is added to offset manager + * @node: Node to be checked + * + * RETURNS: + * true iff the node was previously allocated an offset and added to + * an vma offset manager. + */ +static inline bool drm_vma_node_has_offset(struct drm_vma_offset_node *node) +{ + return drm_mm_node_allocated(&node->vm_node); +} + +/** + * drm_vma_node_offset_addr() - Return sanitized offset for user-space mmaps + * @node: Linked offset node + * + * Same as drm_vma_node_start() but returns the address as a valid offset that + * can be used for user-space mappings during mmap(). + * This must not be called on unlinked nodes. + * + * RETURNS: + * Offset of @node for byte-based addressing. 0 if the node does not have an + * object allocated. + */ +static inline __u64 drm_vma_node_offset_addr(struct drm_vma_offset_node *node) +{ + return ((__u64)node->vm_node.start) << PAGE_SHIFT; +} + +#endif /* __DRM_VMA_MANAGER_H__ */
Use the new vma manager instead of the old hashtable. Also convert all drivers to use the new convenience helpers. This drops all the (map_list.hash.key << PAGE_SHIFT) non-sense.
Locking and access-management is exactly the same as before with an additional lock inside of the vma-manager, which strictly wouldn't be needed for gem.
v2: - rebase on drm-next - init nodes via drm_vma_node_reset() in drm_gem.c
Signed-off-by: David Herrmann dh.herrmann@gmail.com --- drivers/gpu/drm/drm_gem.c | 90 ++++++------------------------ drivers/gpu/drm/drm_gem_cma_helper.c | 9 +-- drivers/gpu/drm/exynos/exynos_drm_gem.c | 7 ++- drivers/gpu/drm/gma500/gem.c | 8 +-- drivers/gpu/drm/i915/i915_gem.c | 9 +-- drivers/gpu/drm/omapdrm/omap_gem.c | 11 ++-- drivers/gpu/drm/omapdrm/omap_gem_helpers.c | 49 +--------------- drivers/gpu/drm/udl/udl_gem.c | 6 +- include/drm/drmP.h | 7 +-- include/drm/drm_vma_manager.h | 1 - include/uapi/drm/drm.h | 2 +- 11 files changed, 49 insertions(+), 150 deletions(-)
diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c index 603f256..b5db89b 100644 --- a/drivers/gpu/drm/drm_gem.c +++ b/drivers/gpu/drm/drm_gem.c @@ -37,6 +37,7 @@ #include <linux/shmem_fs.h> #include <linux/dma-buf.h> #include <drm/drmP.h> +#include <drm/drm_vma_manager.h>
/** @file drm_gem.c * @@ -102,14 +103,9 @@ drm_gem_init(struct drm_device *dev) }
dev->mm_private = mm; - - if (drm_ht_create(&mm->offset_hash, 12)) { - kfree(mm); - return -ENOMEM; - } - - drm_mm_init(&mm->offset_manager, DRM_FILE_PAGE_OFFSET_START, - DRM_FILE_PAGE_OFFSET_SIZE); + drm_vma_offset_manager_init(&mm->vma_manager, + DRM_FILE_PAGE_OFFSET_START, + DRM_FILE_PAGE_OFFSET_SIZE);
return 0; } @@ -119,8 +115,7 @@ drm_gem_destroy(struct drm_device *dev) { struct drm_gem_mm *mm = dev->mm_private;
- drm_mm_takedown(&mm->offset_manager); - drm_ht_remove(&mm->offset_hash); + drm_vma_offset_manager_destroy(&mm->vma_manager); kfree(mm); dev->mm_private = NULL; } @@ -141,6 +136,7 @@ int drm_gem_object_init(struct drm_device *dev,
kref_init(&obj->refcount); atomic_set(&obj->handle_count, 0); + drm_vma_node_reset(&obj->vma_node); obj->size = size;
return 0; @@ -162,6 +158,7 @@ int drm_gem_private_object_init(struct drm_device *dev,
kref_init(&obj->refcount); atomic_set(&obj->handle_count, 0); + drm_vma_node_reset(&obj->vma_node); obj->size = size;
return 0; @@ -306,12 +303,8 @@ drm_gem_free_mmap_offset(struct drm_gem_object *obj) { struct drm_device *dev = obj->dev; struct drm_gem_mm *mm = dev->mm_private; - struct drm_map_list *list = &obj->map_list;
- drm_ht_remove_item(&mm->offset_hash, &list->hash); - drm_mm_put_block(list->file_offset_node); - kfree(list->map); - list->map = NULL; + drm_vma_offset_remove(&mm->vma_manager, &obj->vma_node); } EXPORT_SYMBOL(drm_gem_free_mmap_offset);
@@ -331,54 +324,9 @@ drm_gem_create_mmap_offset(struct drm_gem_object *obj) { struct drm_device *dev = obj->dev; struct drm_gem_mm *mm = dev->mm_private; - struct drm_map_list *list; - struct drm_local_map *map; - int ret; - - /* Set the object up for mmap'ing */ - list = &obj->map_list; - list->map = kzalloc(sizeof(struct drm_map_list), GFP_KERNEL); - if (!list->map) - return -ENOMEM; - - map = list->map; - map->type = _DRM_GEM; - map->size = obj->size; - map->handle = obj; - - /* Get a DRM GEM mmap offset allocated... */ - list->file_offset_node = drm_mm_search_free(&mm->offset_manager, - obj->size / PAGE_SIZE, 0, false); - - if (!list->file_offset_node) { - DRM_ERROR("failed to allocate offset for bo %d\n", obj->name); - ret = -ENOSPC; - goto out_free_list; - }
- list->file_offset_node = drm_mm_get_block(list->file_offset_node, - obj->size / PAGE_SIZE, 0); - if (!list->file_offset_node) { - ret = -ENOMEM; - goto out_free_list; - } - - list->hash.key = list->file_offset_node->start; - ret = drm_ht_insert_item(&mm->offset_hash, &list->hash); - if (ret) { - DRM_ERROR("failed to add to map hash\n"); - goto out_free_mm; - } - - return 0; - -out_free_mm: - drm_mm_put_block(list->file_offset_node); -out_free_list: - kfree(list->map); - list->map = NULL; - - return ret; + return drm_vma_offset_add(&mm->vma_manager, &obj->vma_node, + obj->size / PAGE_SIZE); } EXPORT_SYMBOL(drm_gem_create_mmap_offset);
@@ -707,8 +655,8 @@ int drm_gem_mmap(struct file *filp, struct vm_area_struct *vma) struct drm_file *priv = filp->private_data; struct drm_device *dev = priv->minor->dev; struct drm_gem_mm *mm = dev->mm_private; - struct drm_local_map *map = NULL; - struct drm_hash_item *hash; + struct drm_gem_object *obj; + struct drm_vma_offset_node *node; int ret = 0;
if (drm_device_is_unplugged(dev)) @@ -716,21 +664,15 @@ int drm_gem_mmap(struct file *filp, struct vm_area_struct *vma)
mutex_lock(&dev->struct_mutex);
- if (drm_ht_find_item(&mm->offset_hash, vma->vm_pgoff, &hash)) { + node = drm_vma_offset_exact_lookup(&mm->vma_manager, vma->vm_pgoff); + if (!node) { mutex_unlock(&dev->struct_mutex); return drm_mmap(filp, vma); }
- map = drm_hash_entry(hash, struct drm_map_list, hash)->map; - if (!map || - ((map->flags & _DRM_RESTRICTED) && !capable(CAP_SYS_ADMIN))) { - ret = -EPERM; - goto out_unlock; - } - - ret = drm_gem_mmap_obj(map->handle, map->size, vma); + obj = container_of(node, struct drm_gem_object, vma_node); + ret = drm_gem_mmap_obj(obj, node->vm_pages, vma);
-out_unlock: mutex_unlock(&dev->struct_mutex);
return ret; diff --git a/drivers/gpu/drm/drm_gem_cma_helper.c b/drivers/gpu/drm/drm_gem_cma_helper.c index ce06397..2e0080b 100644 --- a/drivers/gpu/drm/drm_gem_cma_helper.c +++ b/drivers/gpu/drm/drm_gem_cma_helper.c @@ -27,10 +27,11 @@ #include <drm/drmP.h> #include <drm/drm.h> #include <drm/drm_gem_cma_helper.h> +#include <drm/drm_vma_manager.h>
static unsigned int get_gem_mmap_offset(struct drm_gem_object *obj) { - return (unsigned int)obj->map_list.hash.key << PAGE_SHIFT; + return (unsigned int)drm_vma_node_offset_addr(&obj->vma_node); }
/* @@ -172,7 +173,7 @@ void drm_gem_cma_free_object(struct drm_gem_object *gem_obj) { struct drm_gem_cma_object *cma_obj;
- if (gem_obj->map_list.map) + if (drm_vma_node_has_offset(&gem_obj->vma_node)) drm_gem_free_mmap_offset(gem_obj);
cma_obj = to_drm_gem_cma_obj(gem_obj); @@ -305,8 +306,8 @@ void drm_gem_cma_describe(struct drm_gem_cma_object *cma_obj, struct seq_file *m
WARN_ON(!mutex_is_locked(&dev->struct_mutex));
- if (obj->map_list.map) - off = (uint64_t)obj->map_list.hash.key; + if (drm_vma_node_has_offset(&obj->vma_node)) + off = drm_vma_node_start(&obj->vma_node);
seq_printf(m, "%2d (%2d) %08llx %08Zx %p %d", obj->name, obj->refcount.refcount.counter, diff --git a/drivers/gpu/drm/exynos/exynos_drm_gem.c b/drivers/gpu/drm/exynos/exynos_drm_gem.c index c3f15e7..4756513 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_gem.c +++ b/drivers/gpu/drm/exynos/exynos_drm_gem.c @@ -10,6 +10,7 @@ */
#include <drm/drmP.h> +#include <drm/drm_vma_manager.h>
#include <linux/shmem_fs.h> #include <drm/exynos_drm.h> @@ -152,7 +153,7 @@ out: exynos_drm_fini_buf(obj->dev, buf); exynos_gem_obj->buffer = NULL;
- if (obj->map_list.map) + if (drm_vma_node_has_offset(&obj->vma_node)) drm_gem_free_mmap_offset(obj);
/* release file pointer to gem object. */ @@ -702,13 +703,13 @@ int exynos_drm_gem_dumb_map_offset(struct drm_file *file_priv, goto unlock; }
- if (!obj->map_list.map) { + if (!drm_vma_node_has_offset(&obj->vma_node)) { ret = drm_gem_create_mmap_offset(obj); if (ret) goto out; }
- *offset = (u64)obj->map_list.hash.key << PAGE_SHIFT; + *offset = drm_vma_node_offset_addr(&obj->vma_node); DRM_DEBUG_KMS("offset = 0x%lx\n", (unsigned long)*offset);
out: diff --git a/drivers/gpu/drm/gma500/gem.c b/drivers/gpu/drm/gma500/gem.c index eefd6cc..9fd0c9f 100644 --- a/drivers/gpu/drm/gma500/gem.c +++ b/drivers/gpu/drm/gma500/gem.c @@ -26,6 +26,7 @@ #include <drm/drmP.h> #include <drm/drm.h> #include <drm/gma_drm.h> +#include <drm/drm_vma_manager.h> #include "psb_drv.h"
int psb_gem_init_object(struct drm_gem_object *obj) @@ -38,7 +39,7 @@ void psb_gem_free_object(struct drm_gem_object *obj) struct gtt_range *gtt = container_of(obj, struct gtt_range, gem);
/* Remove the list map if one is present */ - if (obj->map_list.map) + if (drm_vma_node_has_offset(&obj->vma_node)) drm_gem_free_mmap_offset(obj); drm_gem_object_release(obj);
@@ -81,13 +82,12 @@ int psb_gem_dumb_map_gtt(struct drm_file *file, struct drm_device *dev, /* What validation is needed here ? */
/* Make it mmapable */ - if (!obj->map_list.map) { + if (!drm_vma_node_has_offset(&obj->vma_node)) { ret = drm_gem_create_mmap_offset(obj); if (ret) goto out; } - /* GEM should really work out the hash offsets for us */ - *offset = (u64)obj->map_list.hash.key << PAGE_SHIFT; + *offset = drm_vma_node_offset_addr(&obj->vma_node); out: drm_gem_object_unreference(obj); unlock: diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 769f752..ca1ceb0 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -26,6 +26,7 @@ */
#include <drm/drmP.h> +#include <drm/drm_vma_manager.h> #include <drm/i915_drm.h> #include "i915_drv.h" #include "i915_trace.h" @@ -1427,7 +1428,7 @@ i915_gem_release_mmap(struct drm_i915_gem_object *obj)
if (obj->base.dev->dev_mapping) unmap_mapping_range(obj->base.dev->dev_mapping, - (loff_t)obj->base.map_list.hash.key<<PAGE_SHIFT, + (loff_t)drm_vma_node_offset_addr(&obj->base.vma_node), obj->base.size, 1);
obj->fault_mappable = false; @@ -1485,7 +1486,7 @@ static int i915_gem_object_create_mmap_offset(struct drm_i915_gem_object *obj) struct drm_i915_private *dev_priv = obj->base.dev->dev_private; int ret;
- if (obj->base.map_list.map) + if (drm_vma_node_has_offset(&obj->base.vma_node)) return 0;
dev_priv->mm.shrinker_no_lock_stealing = true; @@ -1516,7 +1517,7 @@ out:
static void i915_gem_object_free_mmap_offset(struct drm_i915_gem_object *obj) { - if (!obj->base.map_list.map) + if (!drm_vma_node_has_offset(&obj->base.vma_node)) return;
drm_gem_free_mmap_offset(&obj->base); @@ -1557,7 +1558,7 @@ i915_gem_mmap_gtt(struct drm_file *file, if (ret) goto out;
- *offset = (u64)obj->base.map_list.hash.key << PAGE_SHIFT; + *offset = drm_vma_node_offset_addr(&obj->base.vma_node);
out: drm_gem_object_unreference(&obj->base); diff --git a/drivers/gpu/drm/omapdrm/omap_gem.c b/drivers/gpu/drm/omapdrm/omap_gem.c index ebbdf41..0bcfdb9 100644 --- a/drivers/gpu/drm/omapdrm/omap_gem.c +++ b/drivers/gpu/drm/omapdrm/omap_gem.c @@ -20,6 +20,7 @@
#include <linux/spinlock.h> #include <linux/shmem_fs.h> +#include <drm/drm_vma_manager.h>
#include "omap_drv.h" #include "omap_dmm_tiler.h" @@ -311,7 +312,7 @@ static uint64_t mmap_offset(struct drm_gem_object *obj)
WARN_ON(!mutex_is_locked(&dev->struct_mutex));
- if (!obj->map_list.map) { + if (!drm_vma_node_has_offset(&obj->vma_node)) { /* Make it mmapable */ size_t size = omap_gem_mmap_size(obj); int ret = _drm_gem_create_mmap_offset_size(obj, size); @@ -322,7 +323,7 @@ static uint64_t mmap_offset(struct drm_gem_object *obj) } }
- return (uint64_t)obj->map_list.hash.key << PAGE_SHIFT; + return drm_vma_node_offset_addr(&obj->vma_node); }
uint64_t omap_gem_mmap_offset(struct drm_gem_object *obj) @@ -1001,8 +1002,8 @@ void omap_gem_describe(struct drm_gem_object *obj, struct seq_file *m)
WARN_ON(!mutex_is_locked(&dev->struct_mutex));
- if (obj->map_list.map) - off = (uint64_t)obj->map_list.hash.key; + if (drm_vma_node_has_offset(&obj->vma_node)) + off = drm_vma_node_start(&obj->vma_node);
seq_printf(m, "%08x: %2d (%2d) %08llx %08Zx (%2d) %p %4d", omap_obj->flags, obj->name, obj->refcount.refcount.counter, @@ -1309,7 +1310,7 @@ void omap_gem_free_object(struct drm_gem_object *obj)
list_del(&omap_obj->mm_list);
- if (obj->map_list.map) + if (drm_vma_node_has_offset(&obj->vma_node)) drm_gem_free_mmap_offset(obj);
/* this means the object is still pinned.. which really should diff --git a/drivers/gpu/drm/omapdrm/omap_gem_helpers.c b/drivers/gpu/drm/omapdrm/omap_gem_helpers.c index f9eb679..dbb1575 100644 --- a/drivers/gpu/drm/omapdrm/omap_gem_helpers.c +++ b/drivers/gpu/drm/omapdrm/omap_gem_helpers.c @@ -118,52 +118,7 @@ _drm_gem_create_mmap_offset_size(struct drm_gem_object *obj, size_t size) { struct drm_device *dev = obj->dev; struct drm_gem_mm *mm = dev->mm_private; - struct drm_map_list *list; - struct drm_local_map *map; - int ret = 0; - - /* Set the object up for mmap'ing */ - list = &obj->map_list; - list->map = kzalloc(sizeof(struct drm_map_list), GFP_KERNEL); - if (!list->map) - return -ENOMEM; - - map = list->map; - map->type = _DRM_GEM; - map->size = size; - map->handle = obj; - - /* Get a DRM GEM mmap offset allocated... */ - list->file_offset_node = drm_mm_search_free(&mm->offset_manager, - size / PAGE_SIZE, 0, 0); - - if (!list->file_offset_node) { - DRM_ERROR("failed to allocate offset for bo %d\n", obj->name); - ret = -ENOSPC; - goto out_free_list; - } - - list->file_offset_node = drm_mm_get_block(list->file_offset_node, - size / PAGE_SIZE, 0); - if (!list->file_offset_node) { - ret = -ENOMEM; - goto out_free_list; - } - - list->hash.key = list->file_offset_node->start; - ret = drm_ht_insert_item(&mm->offset_hash, &list->hash); - if (ret) { - DRM_ERROR("failed to add to map hash\n"); - goto out_free_mm; - } - - return 0; - -out_free_mm: - drm_mm_put_block(list->file_offset_node); -out_free_list: - kfree(list->map); - list->map = NULL;
- return ret; + return drm_vma_offset_add(&mm->vma_manager, &obj->vma_node, + size / PAGE_SIZE); } diff --git a/drivers/gpu/drm/udl/udl_gem.c b/drivers/gpu/drm/udl/udl_gem.c index ef034fa..59aee8a 100644 --- a/drivers/gpu/drm/udl/udl_gem.c +++ b/drivers/gpu/drm/udl/udl_gem.c @@ -223,7 +223,7 @@ void udl_gem_free_object(struct drm_gem_object *gem_obj) if (obj->pages) udl_gem_put_pages(obj);
- if (gem_obj->map_list.map) + if (drm_vma_node_has_offset(&gem_obj->vma_node)) drm_gem_free_mmap_offset(gem_obj); }
@@ -247,13 +247,13 @@ int udl_gem_mmap(struct drm_file *file, struct drm_device *dev, ret = udl_gem_get_pages(gobj, GFP_KERNEL); if (ret) goto out; - if (!gobj->base.map_list.map) { + if (!drm_vma_node_has_offset(&gobj->base.vma_node)) { ret = drm_gem_create_mmap_offset(obj); if (ret) goto out; }
- *offset = (u64)gobj->base.map_list.hash.key << PAGE_SHIFT; + *offset = drm_vma_node_offset_addr(&gobj->base.vma_node);
out: drm_gem_object_unreference(&gobj->base); diff --git a/include/drm/drmP.h b/include/drm/drmP.h index 82670ac..6544450 100644 --- a/include/drm/drmP.h +++ b/include/drm/drmP.h @@ -71,6 +71,7 @@ #include <asm/pgalloc.h> #include <drm/drm.h> #include <drm/drm_sarea.h> +#include <drm/drm_vma_manager.h>
#include <linux/idr.h>
@@ -587,7 +588,6 @@ struct drm_map_list { struct drm_local_map *map; /**< mapping */ uint64_t user_token; struct drm_master *master; - struct drm_mm_node *file_offset_node; /**< fake offset */ };
/** @@ -622,8 +622,7 @@ struct drm_ati_pcigart_info { * GEM specific mm private for tracking GEM objects */ struct drm_gem_mm { - struct drm_mm offset_manager; /**< Offset mgmt for buffer objects */ - struct drm_open_hash offset_hash; /**< User token hash table for maps */ + struct drm_vma_offset_manager vma_manager; };
/** @@ -644,7 +643,7 @@ struct drm_gem_object { struct file *filp;
/* Mapping info for this object */ - struct drm_map_list map_list; + struct drm_vma_offset_node vma_node;
/** * Size of the object, in bytes. Immutable over the object's diff --git a/include/drm/drm_vma_manager.h b/include/drm/drm_vma_manager.h index b519d7f..4fcc441 100644 --- a/include/drm/drm_vma_manager.h +++ b/include/drm/drm_vma_manager.h @@ -23,7 +23,6 @@ * OTHER DEALINGS IN THE SOFTWARE. */
-#include <drm/drmP.h> #include <drm/drm_mm.h> #include <linux/module.h> #include <linux/rbtree.h> diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h index 238a166..272580c 100644 --- a/include/uapi/drm/drm.h +++ b/include/uapi/drm/drm.h @@ -181,7 +181,7 @@ enum drm_map_type { _DRM_AGP = 3, /**< AGP/GART */ _DRM_SCATTER_GATHER = 4, /**< Scatter/gather memory for PCI DMA */ _DRM_CONSISTENT = 5, /**< Consistent memory for PCI DMA */ - _DRM_GEM = 6, /**< GEM object */ + _DRM_GEM = 6, /**< GEM object (obsolete) */ };
/**
Hi
On Sun, Jul 7, 2013 at 7:17 PM, David Herrmann dh.herrmann@gmail.com wrote:
Use the new vma manager instead of the old hashtable. Also convert all drivers to use the new convenience helpers. This drops all the (map_list.hash.key << PAGE_SHIFT) non-sense.
Locking and access-management is exactly the same as before with an additional lock inside of the vma-manager, which strictly wouldn't be needed for gem.
v2:
- rebase on drm-next
- init nodes via drm_vma_node_reset() in drm_gem.c
I missed the tegra driver and any staging driver. I will fix that in v3 and reduce the series to a minimum.
Regards David
Signed-off-by: David Herrmann dh.herrmann@gmail.com
drivers/gpu/drm/drm_gem.c | 90 ++++++------------------------ drivers/gpu/drm/drm_gem_cma_helper.c | 9 +-- drivers/gpu/drm/exynos/exynos_drm_gem.c | 7 ++- drivers/gpu/drm/gma500/gem.c | 8 +-- drivers/gpu/drm/i915/i915_gem.c | 9 +-- drivers/gpu/drm/omapdrm/omap_gem.c | 11 ++-- drivers/gpu/drm/omapdrm/omap_gem_helpers.c | 49 +--------------- drivers/gpu/drm/udl/udl_gem.c | 6 +- include/drm/drmP.h | 7 +-- include/drm/drm_vma_manager.h | 1 - include/uapi/drm/drm.h | 2 +- 11 files changed, 49 insertions(+), 150 deletions(-)
diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c index 603f256..b5db89b 100644 --- a/drivers/gpu/drm/drm_gem.c +++ b/drivers/gpu/drm/drm_gem.c @@ -37,6 +37,7 @@ #include <linux/shmem_fs.h> #include <linux/dma-buf.h> #include <drm/drmP.h> +#include <drm/drm_vma_manager.h>
/** @file drm_gem.c
@@ -102,14 +103,9 @@ drm_gem_init(struct drm_device *dev) }
dev->mm_private = mm;
if (drm_ht_create(&mm->offset_hash, 12)) {kfree(mm);return -ENOMEM;}drm_mm_init(&mm->offset_manager, DRM_FILE_PAGE_OFFSET_START,DRM_FILE_PAGE_OFFSET_SIZE);
drm_vma_offset_manager_init(&mm->vma_manager,DRM_FILE_PAGE_OFFSET_START,DRM_FILE_PAGE_OFFSET_SIZE); return 0;} @@ -119,8 +115,7 @@ drm_gem_destroy(struct drm_device *dev) { struct drm_gem_mm *mm = dev->mm_private;
drm_mm_takedown(&mm->offset_manager);drm_ht_remove(&mm->offset_hash);
drm_vma_offset_manager_destroy(&mm->vma_manager); kfree(mm); dev->mm_private = NULL;} @@ -141,6 +136,7 @@ int drm_gem_object_init(struct drm_device *dev,
kref_init(&obj->refcount); atomic_set(&obj->handle_count, 0);
drm_vma_node_reset(&obj->vma_node); obj->size = size; return 0;@@ -162,6 +158,7 @@ int drm_gem_private_object_init(struct drm_device *dev,
kref_init(&obj->refcount); atomic_set(&obj->handle_count, 0);
drm_vma_node_reset(&obj->vma_node); obj->size = size; return 0;@@ -306,12 +303,8 @@ drm_gem_free_mmap_offset(struct drm_gem_object *obj) { struct drm_device *dev = obj->dev; struct drm_gem_mm *mm = dev->mm_private;
struct drm_map_list *list = &obj->map_list;drm_ht_remove_item(&mm->offset_hash, &list->hash);drm_mm_put_block(list->file_offset_node);kfree(list->map);list->map = NULL;
drm_vma_offset_remove(&mm->vma_manager, &obj->vma_node);} EXPORT_SYMBOL(drm_gem_free_mmap_offset);
@@ -331,54 +324,9 @@ drm_gem_create_mmap_offset(struct drm_gem_object *obj) { struct drm_device *dev = obj->dev; struct drm_gem_mm *mm = dev->mm_private;
struct drm_map_list *list;struct drm_local_map *map;int ret;/* Set the object up for mmap'ing */list = &obj->map_list;list->map = kzalloc(sizeof(struct drm_map_list), GFP_KERNEL);if (!list->map)return -ENOMEM;map = list->map;map->type = _DRM_GEM;map->size = obj->size;map->handle = obj;/* Get a DRM GEM mmap offset allocated... */list->file_offset_node = drm_mm_search_free(&mm->offset_manager,obj->size / PAGE_SIZE, 0, false);if (!list->file_offset_node) {DRM_ERROR("failed to allocate offset for bo %d\n", obj->name);ret = -ENOSPC;goto out_free_list;}list->file_offset_node = drm_mm_get_block(list->file_offset_node,obj->size / PAGE_SIZE, 0);if (!list->file_offset_node) {ret = -ENOMEM;goto out_free_list;}list->hash.key = list->file_offset_node->start;ret = drm_ht_insert_item(&mm->offset_hash, &list->hash);if (ret) {DRM_ERROR("failed to add to map hash\n");goto out_free_mm;}return 0;-out_free_mm:
drm_mm_put_block(list->file_offset_node);-out_free_list:
kfree(list->map);list->map = NULL;return ret;
return drm_vma_offset_add(&mm->vma_manager, &obj->vma_node,obj->size / PAGE_SIZE);} EXPORT_SYMBOL(drm_gem_create_mmap_offset);
@@ -707,8 +655,8 @@ int drm_gem_mmap(struct file *filp, struct vm_area_struct *vma) struct drm_file *priv = filp->private_data; struct drm_device *dev = priv->minor->dev; struct drm_gem_mm *mm = dev->mm_private;
struct drm_local_map *map = NULL;struct drm_hash_item *hash;
struct drm_gem_object *obj;struct drm_vma_offset_node *node; int ret = 0; if (drm_device_is_unplugged(dev))@@ -716,21 +664,15 @@ int drm_gem_mmap(struct file *filp, struct vm_area_struct *vma)
mutex_lock(&dev->struct_mutex);
if (drm_ht_find_item(&mm->offset_hash, vma->vm_pgoff, &hash)) {
node = drm_vma_offset_exact_lookup(&mm->vma_manager, vma->vm_pgoff);if (!node) { mutex_unlock(&dev->struct_mutex); return drm_mmap(filp, vma); }
map = drm_hash_entry(hash, struct drm_map_list, hash)->map;if (!map ||((map->flags & _DRM_RESTRICTED) && !capable(CAP_SYS_ADMIN))) {ret = -EPERM;goto out_unlock;}ret = drm_gem_mmap_obj(map->handle, map->size, vma);
obj = container_of(node, struct drm_gem_object, vma_node);ret = drm_gem_mmap_obj(obj, node->vm_pages, vma);-out_unlock: mutex_unlock(&dev->struct_mutex);
return ret;diff --git a/drivers/gpu/drm/drm_gem_cma_helper.c b/drivers/gpu/drm/drm_gem_cma_helper.c index ce06397..2e0080b 100644 --- a/drivers/gpu/drm/drm_gem_cma_helper.c +++ b/drivers/gpu/drm/drm_gem_cma_helper.c @@ -27,10 +27,11 @@ #include <drm/drmP.h> #include <drm/drm.h> #include <drm/drm_gem_cma_helper.h> +#include <drm/drm_vma_manager.h>
static unsigned int get_gem_mmap_offset(struct drm_gem_object *obj) {
return (unsigned int)obj->map_list.hash.key << PAGE_SHIFT;
return (unsigned int)drm_vma_node_offset_addr(&obj->vma_node);}
/* @@ -172,7 +173,7 @@ void drm_gem_cma_free_object(struct drm_gem_object *gem_obj) { struct drm_gem_cma_object *cma_obj;
if (gem_obj->map_list.map)
if (drm_vma_node_has_offset(&gem_obj->vma_node)) drm_gem_free_mmap_offset(gem_obj); cma_obj = to_drm_gem_cma_obj(gem_obj);@@ -305,8 +306,8 @@ void drm_gem_cma_describe(struct drm_gem_cma_object *cma_obj, struct seq_file *m
WARN_ON(!mutex_is_locked(&dev->struct_mutex));
if (obj->map_list.map)off = (uint64_t)obj->map_list.hash.key;
if (drm_vma_node_has_offset(&obj->vma_node))off = drm_vma_node_start(&obj->vma_node); seq_printf(m, "%2d (%2d) %08llx %08Zx %p %d", obj->name, obj->refcount.refcount.counter,diff --git a/drivers/gpu/drm/exynos/exynos_drm_gem.c b/drivers/gpu/drm/exynos/exynos_drm_gem.c index c3f15e7..4756513 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_gem.c +++ b/drivers/gpu/drm/exynos/exynos_drm_gem.c @@ -10,6 +10,7 @@ */
#include <drm/drmP.h> +#include <drm/drm_vma_manager.h>
#include <linux/shmem_fs.h> #include <drm/exynos_drm.h> @@ -152,7 +153,7 @@ out: exynos_drm_fini_buf(obj->dev, buf); exynos_gem_obj->buffer = NULL;
if (obj->map_list.map)
if (drm_vma_node_has_offset(&obj->vma_node)) drm_gem_free_mmap_offset(obj); /* release file pointer to gem object. */@@ -702,13 +703,13 @@ int exynos_drm_gem_dumb_map_offset(struct drm_file *file_priv, goto unlock; }
if (!obj->map_list.map) {
if (!drm_vma_node_has_offset(&obj->vma_node)) { ret = drm_gem_create_mmap_offset(obj); if (ret) goto out; }
*offset = (u64)obj->map_list.hash.key << PAGE_SHIFT;
*offset = drm_vma_node_offset_addr(&obj->vma_node); DRM_DEBUG_KMS("offset = 0x%lx\n", (unsigned long)*offset);out: diff --git a/drivers/gpu/drm/gma500/gem.c b/drivers/gpu/drm/gma500/gem.c index eefd6cc..9fd0c9f 100644 --- a/drivers/gpu/drm/gma500/gem.c +++ b/drivers/gpu/drm/gma500/gem.c @@ -26,6 +26,7 @@ #include <drm/drmP.h> #include <drm/drm.h> #include <drm/gma_drm.h> +#include <drm/drm_vma_manager.h> #include "psb_drv.h"
int psb_gem_init_object(struct drm_gem_object *obj) @@ -38,7 +39,7 @@ void psb_gem_free_object(struct drm_gem_object *obj) struct gtt_range *gtt = container_of(obj, struct gtt_range, gem);
/* Remove the list map if one is present */
if (obj->map_list.map)
if (drm_vma_node_has_offset(&obj->vma_node)) drm_gem_free_mmap_offset(obj); drm_gem_object_release(obj);@@ -81,13 +82,12 @@ int psb_gem_dumb_map_gtt(struct drm_file *file, struct drm_device *dev, /* What validation is needed here ? */
/* Make it mmapable */
if (!obj->map_list.map) {
if (!drm_vma_node_has_offset(&obj->vma_node)) { ret = drm_gem_create_mmap_offset(obj); if (ret) goto out; }
/* GEM should really work out the hash offsets for us */*offset = (u64)obj->map_list.hash.key << PAGE_SHIFT;
*offset = drm_vma_node_offset_addr(&obj->vma_node);out: drm_gem_object_unreference(obj); unlock: diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 769f752..ca1ceb0 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -26,6 +26,7 @@ */
#include <drm/drmP.h> +#include <drm/drm_vma_manager.h> #include <drm/i915_drm.h> #include "i915_drv.h" #include "i915_trace.h" @@ -1427,7 +1428,7 @@ i915_gem_release_mmap(struct drm_i915_gem_object *obj)
if (obj->base.dev->dev_mapping) unmap_mapping_range(obj->base.dev->dev_mapping,
(loff_t)obj->base.map_list.hash.key<<PAGE_SHIFT,
(loff_t)drm_vma_node_offset_addr(&obj->base.vma_node), obj->base.size, 1); obj->fault_mappable = false;@@ -1485,7 +1486,7 @@ static int i915_gem_object_create_mmap_offset(struct drm_i915_gem_object *obj) struct drm_i915_private *dev_priv = obj->base.dev->dev_private; int ret;
if (obj->base.map_list.map)
if (drm_vma_node_has_offset(&obj->base.vma_node)) return 0; dev_priv->mm.shrinker_no_lock_stealing = true;@@ -1516,7 +1517,7 @@ out:
static void i915_gem_object_free_mmap_offset(struct drm_i915_gem_object *obj) {
if (!obj->base.map_list.map)
if (!drm_vma_node_has_offset(&obj->base.vma_node)) return; drm_gem_free_mmap_offset(&obj->base);@@ -1557,7 +1558,7 @@ i915_gem_mmap_gtt(struct drm_file *file, if (ret) goto out;
*offset = (u64)obj->base.map_list.hash.key << PAGE_SHIFT;
*offset = drm_vma_node_offset_addr(&obj->base.vma_node);out: drm_gem_object_unreference(&obj->base); diff --git a/drivers/gpu/drm/omapdrm/omap_gem.c b/drivers/gpu/drm/omapdrm/omap_gem.c index ebbdf41..0bcfdb9 100644 --- a/drivers/gpu/drm/omapdrm/omap_gem.c +++ b/drivers/gpu/drm/omapdrm/omap_gem.c @@ -20,6 +20,7 @@
#include <linux/spinlock.h> #include <linux/shmem_fs.h> +#include <drm/drm_vma_manager.h>
#include "omap_drv.h" #include "omap_dmm_tiler.h" @@ -311,7 +312,7 @@ static uint64_t mmap_offset(struct drm_gem_object *obj)
WARN_ON(!mutex_is_locked(&dev->struct_mutex));
if (!obj->map_list.map) {
if (!drm_vma_node_has_offset(&obj->vma_node)) { /* Make it mmapable */ size_t size = omap_gem_mmap_size(obj); int ret = _drm_gem_create_mmap_offset_size(obj, size);@@ -322,7 +323,7 @@ static uint64_t mmap_offset(struct drm_gem_object *obj) } }
return (uint64_t)obj->map_list.hash.key << PAGE_SHIFT;
return drm_vma_node_offset_addr(&obj->vma_node);}
uint64_t omap_gem_mmap_offset(struct drm_gem_object *obj) @@ -1001,8 +1002,8 @@ void omap_gem_describe(struct drm_gem_object *obj, struct seq_file *m)
WARN_ON(!mutex_is_locked(&dev->struct_mutex));
if (obj->map_list.map)off = (uint64_t)obj->map_list.hash.key;
if (drm_vma_node_has_offset(&obj->vma_node))off = drm_vma_node_start(&obj->vma_node); seq_printf(m, "%08x: %2d (%2d) %08llx %08Zx (%2d) %p %4d", omap_obj->flags, obj->name, obj->refcount.refcount.counter,@@ -1309,7 +1310,7 @@ void omap_gem_free_object(struct drm_gem_object *obj)
list_del(&omap_obj->mm_list);
if (obj->map_list.map)
if (drm_vma_node_has_offset(&obj->vma_node)) drm_gem_free_mmap_offset(obj); /* this means the object is still pinned.. which really shoulddiff --git a/drivers/gpu/drm/omapdrm/omap_gem_helpers.c b/drivers/gpu/drm/omapdrm/omap_gem_helpers.c index f9eb679..dbb1575 100644 --- a/drivers/gpu/drm/omapdrm/omap_gem_helpers.c +++ b/drivers/gpu/drm/omapdrm/omap_gem_helpers.c @@ -118,52 +118,7 @@ _drm_gem_create_mmap_offset_size(struct drm_gem_object *obj, size_t size) { struct drm_device *dev = obj->dev; struct drm_gem_mm *mm = dev->mm_private;
struct drm_map_list *list;struct drm_local_map *map;int ret = 0;/* Set the object up for mmap'ing */list = &obj->map_list;list->map = kzalloc(sizeof(struct drm_map_list), GFP_KERNEL);if (!list->map)return -ENOMEM;map = list->map;map->type = _DRM_GEM;map->size = size;map->handle = obj;/* Get a DRM GEM mmap offset allocated... */list->file_offset_node = drm_mm_search_free(&mm->offset_manager,size / PAGE_SIZE, 0, 0);if (!list->file_offset_node) {DRM_ERROR("failed to allocate offset for bo %d\n", obj->name);ret = -ENOSPC;goto out_free_list;}list->file_offset_node = drm_mm_get_block(list->file_offset_node,size / PAGE_SIZE, 0);if (!list->file_offset_node) {ret = -ENOMEM;goto out_free_list;}list->hash.key = list->file_offset_node->start;ret = drm_ht_insert_item(&mm->offset_hash, &list->hash);if (ret) {DRM_ERROR("failed to add to map hash\n");goto out_free_mm;}return 0;-out_free_mm:
drm_mm_put_block(list->file_offset_node);-out_free_list:
kfree(list->map);list->map = NULL;return ret;
return drm_vma_offset_add(&mm->vma_manager, &obj->vma_node,size / PAGE_SIZE);} diff --git a/drivers/gpu/drm/udl/udl_gem.c b/drivers/gpu/drm/udl/udl_gem.c index ef034fa..59aee8a 100644 --- a/drivers/gpu/drm/udl/udl_gem.c +++ b/drivers/gpu/drm/udl/udl_gem.c @@ -223,7 +223,7 @@ void udl_gem_free_object(struct drm_gem_object *gem_obj) if (obj->pages) udl_gem_put_pages(obj);
if (gem_obj->map_list.map)
if (drm_vma_node_has_offset(&gem_obj->vma_node)) drm_gem_free_mmap_offset(gem_obj);}
@@ -247,13 +247,13 @@ int udl_gem_mmap(struct drm_file *file, struct drm_device *dev, ret = udl_gem_get_pages(gobj, GFP_KERNEL); if (ret) goto out;
if (!gobj->base.map_list.map) {
if (!drm_vma_node_has_offset(&gobj->base.vma_node)) { ret = drm_gem_create_mmap_offset(obj); if (ret) goto out; }
*offset = (u64)gobj->base.map_list.hash.key << PAGE_SHIFT;
*offset = drm_vma_node_offset_addr(&gobj->base.vma_node);out: drm_gem_object_unreference(&gobj->base); diff --git a/include/drm/drmP.h b/include/drm/drmP.h index 82670ac..6544450 100644 --- a/include/drm/drmP.h +++ b/include/drm/drmP.h @@ -71,6 +71,7 @@ #include <asm/pgalloc.h> #include <drm/drm.h> #include <drm/drm_sarea.h> +#include <drm/drm_vma_manager.h>
#include <linux/idr.h>
@@ -587,7 +588,6 @@ struct drm_map_list { struct drm_local_map *map; /**< mapping */ uint64_t user_token; struct drm_master *master;
struct drm_mm_node *file_offset_node; /**< fake offset */};
/** @@ -622,8 +622,7 @@ struct drm_ati_pcigart_info {
- GEM specific mm private for tracking GEM objects
*/ struct drm_gem_mm {
struct drm_mm offset_manager; /**< Offset mgmt for buffer objects */struct drm_open_hash offset_hash; /**< User token hash table for maps */
struct drm_vma_offset_manager vma_manager;};
/** @@ -644,7 +643,7 @@ struct drm_gem_object { struct file *filp;
/* Mapping info for this object */
struct drm_map_list map_list;
struct drm_vma_offset_node vma_node; /** * Size of the object, in bytes. Immutable over the object'sdiff --git a/include/drm/drm_vma_manager.h b/include/drm/drm_vma_manager.h index b519d7f..4fcc441 100644 --- a/include/drm/drm_vma_manager.h +++ b/include/drm/drm_vma_manager.h @@ -23,7 +23,6 @@
- OTHER DEALINGS IN THE SOFTWARE.
*/
-#include <drm/drmP.h> #include <drm/drm_mm.h> #include <linux/module.h> #include <linux/rbtree.h> diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h index 238a166..272580c 100644 --- a/include/uapi/drm/drm.h +++ b/include/uapi/drm/drm.h @@ -181,7 +181,7 @@ enum drm_map_type { _DRM_AGP = 3, /**< AGP/GART */ _DRM_SCATTER_GATHER = 4, /**< Scatter/gather memory for PCI DMA */ _DRM_CONSISTENT = 5, /**< Consistent memory for PCI DMA */
_DRM_GEM = 6, /**< GEM object */
_DRM_GEM = 6, /**< GEM object (obsolete) */};
/**
1.8.3.2
Use the new vma-manager infrastructure. This doesn't change any implementation details as the vma-offset-manager is nearly copied 1-to-1 from TTM.
Even though the vma-manager uses its own locks, we still need bo->vm_lock to prevent bos from being destroyed before we can get a reference during lookup. However, this lock is not needed during vm-setup as we still hold a reference there.
This also drops the addr_space_offset member as it is a copy of vm_start in vma_node objects. Use the accessor functions instead.
Signed-off-by: David Herrmann dh.herrmann@gmail.com --- drivers/gpu/drm/ast/ast_main.c | 2 +- drivers/gpu/drm/cirrus/cirrus_main.c | 2 +- drivers/gpu/drm/mgag200/mgag200_main.c | 2 +- drivers/gpu/drm/nouveau/nouveau_display.c | 2 +- drivers/gpu/drm/nouveau/nouveau_gem.c | 2 +- drivers/gpu/drm/qxl/qxl_object.h | 2 +- drivers/gpu/drm/qxl/qxl_release.c | 2 +- drivers/gpu/drm/radeon/radeon_object.h | 5 +- drivers/gpu/drm/ttm/ttm_bo.c | 84 ++++++------------------------- drivers/gpu/drm/ttm/ttm_bo_util.c | 3 +- drivers/gpu/drm/ttm/ttm_bo_vm.c | 81 ++++++++++++----------------- drivers/gpu/drm/vmwgfx/vmwgfx_resource.c | 4 +- include/drm/ttm/ttm_bo_api.h | 15 ++---- include/drm/ttm/ttm_bo_driver.h | 7 +-- 14 files changed, 65 insertions(+), 148 deletions(-)
diff --git a/drivers/gpu/drm/ast/ast_main.c b/drivers/gpu/drm/ast/ast_main.c index f60fd7b..c195dc2 100644 --- a/drivers/gpu/drm/ast/ast_main.c +++ b/drivers/gpu/drm/ast/ast_main.c @@ -487,7 +487,7 @@ void ast_gem_free_object(struct drm_gem_object *obj)
static inline u64 ast_bo_mmap_offset(struct ast_bo *bo) { - return bo->bo.addr_space_offset; + return drm_vma_node_offset_addr(&bo->bo.vma_node); } int ast_dumb_mmap_offset(struct drm_file *file, diff --git a/drivers/gpu/drm/cirrus/cirrus_main.c b/drivers/gpu/drm/cirrus/cirrus_main.c index 35cbae8..3a7a0ef 100644 --- a/drivers/gpu/drm/cirrus/cirrus_main.c +++ b/drivers/gpu/drm/cirrus/cirrus_main.c @@ -294,7 +294,7 @@ void cirrus_gem_free_object(struct drm_gem_object *obj)
static inline u64 cirrus_bo_mmap_offset(struct cirrus_bo *bo) { - return bo->bo.addr_space_offset; + return drm_vma_node_offset_addr(&bo->bo.vma_node); }
int diff --git a/drivers/gpu/drm/mgag200/mgag200_main.c b/drivers/gpu/drm/mgag200/mgag200_main.c index 9fa5685..1a75ea3 100644 --- a/drivers/gpu/drm/mgag200/mgag200_main.c +++ b/drivers/gpu/drm/mgag200/mgag200_main.c @@ -349,7 +349,7 @@ void mgag200_gem_free_object(struct drm_gem_object *obj)
static inline u64 mgag200_bo_mmap_offset(struct mgag200_bo *bo) { - return bo->bo.addr_space_offset; + return drm_vma_node_offset_addr(&bo->bo.vma_node); }
int diff --git a/drivers/gpu/drm/nouveau/nouveau_display.c b/drivers/gpu/drm/nouveau/nouveau_display.c index 708b2d1..7a8caa1 100644 --- a/drivers/gpu/drm/nouveau/nouveau_display.c +++ b/drivers/gpu/drm/nouveau/nouveau_display.c @@ -697,7 +697,7 @@ nouveau_display_dumb_map_offset(struct drm_file *file_priv, gem = drm_gem_object_lookup(dev, file_priv, handle); if (gem) { struct nouveau_bo *bo = gem->driver_private; - *poffset = bo->bo.addr_space_offset; + *poffset = drm_vma_node_offset_addr(&bo->bo.vma_node); drm_gem_object_unreference_unlocked(gem); return 0; } diff --git a/drivers/gpu/drm/nouveau/nouveau_gem.c b/drivers/gpu/drm/nouveau/nouveau_gem.c index e72d09c..86597eb 100644 --- a/drivers/gpu/drm/nouveau/nouveau_gem.c +++ b/drivers/gpu/drm/nouveau/nouveau_gem.c @@ -226,7 +226,7 @@ nouveau_gem_info(struct drm_file *file_priv, struct drm_gem_object *gem, }
rep->size = nvbo->bo.mem.num_pages << PAGE_SHIFT; - rep->map_handle = nvbo->bo.addr_space_offset; + rep->map_handle = drm_vma_node_offset_addr(&nvbo->bo.vma_node); rep->tile_mode = nvbo->tile_mode; rep->tile_flags = nvbo->tile_flags; return 0; diff --git a/drivers/gpu/drm/qxl/qxl_object.h b/drivers/gpu/drm/qxl/qxl_object.h index ee7ad79..af10165 100644 --- a/drivers/gpu/drm/qxl/qxl_object.h +++ b/drivers/gpu/drm/qxl/qxl_object.h @@ -59,7 +59,7 @@ static inline unsigned long qxl_bo_size(struct qxl_bo *bo)
static inline u64 qxl_bo_mmap_offset(struct qxl_bo *bo) { - return bo->tbo.addr_space_offset; + return drm_vma_node_offset_addr(&bo->tbo.vma_node); }
static inline int qxl_bo_wait(struct qxl_bo *bo, u32 *mem_type, diff --git a/drivers/gpu/drm/qxl/qxl_release.c b/drivers/gpu/drm/qxl/qxl_release.c index b443d67..1a648e1 100644 --- a/drivers/gpu/drm/qxl/qxl_release.c +++ b/drivers/gpu/drm/qxl/qxl_release.c @@ -87,7 +87,7 @@ qxl_release_free(struct qxl_device *qdev,
for (i = 0 ; i < release->bo_count; ++i) { QXL_INFO(qdev, "release %llx\n", - release->bos[i]->tbo.addr_space_offset + drm_vma_node_offset_addr(&release->bos[i]->tbo.vma_node) - DRM_FILE_OFFSET); qxl_fence_remove_release(&release->bos[i]->fence, release->id); qxl_bo_unref(&release->bos[i]); diff --git a/drivers/gpu/drm/radeon/radeon_object.h b/drivers/gpu/drm/radeon/radeon_object.h index 91519a5..188c682 100644 --- a/drivers/gpu/drm/radeon/radeon_object.h +++ b/drivers/gpu/drm/radeon/radeon_object.h @@ -113,13 +113,10 @@ static inline unsigned radeon_bo_gpu_page_alignment(struct radeon_bo *bo) * @bo: radeon object for which we query the offset * * Returns mmap offset of the object. - * - * Note: addr_space_offset is constant after ttm bo init thus isn't protected - * by any lock. */ static inline u64 radeon_bo_mmap_offset(struct radeon_bo *bo) { - return bo->tbo.addr_space_offset; + return drm_vma_node_offset_addr(&bo->tbo.vma_node); }
extern int radeon_bo_wait(struct radeon_bo *bo, u32 *mem_type, diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c index cb9dd67..245850b 100644 --- a/drivers/gpu/drm/ttm/ttm_bo.c +++ b/drivers/gpu/drm/ttm/ttm_bo.c @@ -616,11 +616,7 @@ static void ttm_bo_release(struct kref *kref) struct ttm_mem_type_manager *man = &bdev->man[bo->mem.mem_type];
write_lock(&bdev->vm_lock); - if (likely(bo->vm_node != NULL)) { - rb_erase(&bo->vm_rb, &bdev->addr_space_rb); - drm_mm_put_block(bo->vm_node); - bo->vm_node = NULL; - } + drm_vma_offset_remove(&bdev->vma_manager, &bo->vma_node); write_unlock(&bdev->vm_lock); ttm_mem_io_lock(man, false); ttm_mem_io_free_vm(bo); @@ -1129,6 +1125,7 @@ int ttm_bo_init(struct ttm_bo_device *bdev, bo->resv = &bo->ttm_resv; reservation_object_init(bo->resv); atomic_inc(&bo->glob->bo_count); + drm_vma_node_reset(&bo->vma_node);
ret = ttm_bo_check_placement(bo, placement);
@@ -1424,9 +1421,8 @@ int ttm_bo_device_release(struct ttm_bo_device *bdev) TTM_DEBUG("Swap list was clean\n"); spin_unlock(&glob->lru_lock);
- BUG_ON(!drm_mm_clean(&bdev->addr_space_mm)); write_lock(&bdev->vm_lock); - drm_mm_takedown(&bdev->addr_space_mm); + drm_vma_offset_manager_destroy(&bdev->vma_manager); write_unlock(&bdev->vm_lock);
return ret; @@ -1454,9 +1450,8 @@ int ttm_bo_device_init(struct ttm_bo_device *bdev, if (unlikely(ret != 0)) goto out_no_sys;
- bdev->addr_space_rb = RB_ROOT; - drm_mm_init(&bdev->addr_space_mm, file_page_offset, 0x10000000); - + drm_vma_offset_manager_init(&bdev->vma_manager, file_page_offset, + 0x10000000); INIT_DELAYED_WORK(&bdev->wq, ttm_bo_delayed_workqueue); INIT_LIST_HEAD(&bdev->ddestroy); bdev->dev_mapping = NULL; @@ -1498,12 +1493,17 @@ bool ttm_mem_reg_is_pci(struct ttm_bo_device *bdev, struct ttm_mem_reg *mem) void ttm_bo_unmap_virtual_locked(struct ttm_buffer_object *bo) { struct ttm_bo_device *bdev = bo->bdev; - loff_t offset = (loff_t) bo->addr_space_offset; - loff_t holelen = ((loff_t) bo->mem.num_pages) << PAGE_SHIFT; + loff_t offset, holelen;
if (!bdev->dev_mapping) return; - unmap_mapping_range(bdev->dev_mapping, offset, holelen, 1); + + if (drm_vma_node_has_offset(&bo->vma_node)) { + offset = (loff_t) drm_vma_node_offset_addr(&bo->vma_node); + holelen = ((loff_t) bo->mem.num_pages) << PAGE_SHIFT; + + unmap_mapping_range(bdev->dev_mapping, offset, holelen, 1); + } ttm_mem_io_free_vm(bo); }
@@ -1520,31 +1520,6 @@ void ttm_bo_unmap_virtual(struct ttm_buffer_object *bo)
EXPORT_SYMBOL(ttm_bo_unmap_virtual);
-static void ttm_bo_vm_insert_rb(struct ttm_buffer_object *bo) -{ - struct ttm_bo_device *bdev = bo->bdev; - struct rb_node **cur = &bdev->addr_space_rb.rb_node; - struct rb_node *parent = NULL; - struct ttm_buffer_object *cur_bo; - unsigned long offset = bo->vm_node->start; - unsigned long cur_offset; - - while (*cur) { - parent = *cur; - cur_bo = rb_entry(parent, struct ttm_buffer_object, vm_rb); - cur_offset = cur_bo->vm_node->start; - if (offset < cur_offset) - cur = &parent->rb_left; - else if (offset > cur_offset) - cur = &parent->rb_right; - else - BUG(); - } - - rb_link_node(&bo->vm_rb, parent, cur); - rb_insert_color(&bo->vm_rb, &bdev->addr_space_rb); -} - /** * ttm_bo_setup_vm: * @@ -1559,38 +1534,9 @@ static void ttm_bo_vm_insert_rb(struct ttm_buffer_object *bo) static int ttm_bo_setup_vm(struct ttm_buffer_object *bo) { struct ttm_bo_device *bdev = bo->bdev; - int ret; - -retry_pre_get: - ret = drm_mm_pre_get(&bdev->addr_space_mm); - if (unlikely(ret != 0)) - return ret;
- write_lock(&bdev->vm_lock); - bo->vm_node = drm_mm_search_free(&bdev->addr_space_mm, - bo->mem.num_pages, 0, 0); - - if (unlikely(bo->vm_node == NULL)) { - ret = -ENOMEM; - goto out_unlock; - } - - bo->vm_node = drm_mm_get_block_atomic(bo->vm_node, - bo->mem.num_pages, 0); - - if (unlikely(bo->vm_node == NULL)) { - write_unlock(&bdev->vm_lock); - goto retry_pre_get; - } - - ttm_bo_vm_insert_rb(bo); - write_unlock(&bdev->vm_lock); - bo->addr_space_offset = ((uint64_t) bo->vm_node->start) << PAGE_SHIFT; - - return 0; -out_unlock: - write_unlock(&bdev->vm_lock); - return ret; + return drm_vma_offset_add(&bdev->vma_manager, &bo->vma_node, + bo->mem.num_pages); }
int ttm_bo_wait(struct ttm_buffer_object *bo, diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c b/drivers/gpu/drm/ttm/ttm_bo_util.c index 319cf41..7cc904d 100644 --- a/drivers/gpu/drm/ttm/ttm_bo_util.c +++ b/drivers/gpu/drm/ttm/ttm_bo_util.c @@ -30,6 +30,7 @@
#include <drm/ttm/ttm_bo_driver.h> #include <drm/ttm/ttm_placement.h> +#include <drm/drm_vma_manager.h> #include <linux/io.h> #include <linux/highmem.h> #include <linux/wait.h> @@ -450,7 +451,7 @@ static int ttm_buffer_object_transfer(struct ttm_buffer_object *bo, INIT_LIST_HEAD(&fbo->lru); INIT_LIST_HEAD(&fbo->swap); INIT_LIST_HEAD(&fbo->io_reserve_lru); - fbo->vm_node = NULL; + drm_vma_node_reset(&fbo->vma_node); atomic_set(&fbo->cpu_writers, 0);
spin_lock(&bdev->fence_lock); diff --git a/drivers/gpu/drm/ttm/ttm_bo_vm.c b/drivers/gpu/drm/ttm/ttm_bo_vm.c index 3df9f16..54a67f1 100644 --- a/drivers/gpu/drm/ttm/ttm_bo_vm.c +++ b/drivers/gpu/drm/ttm/ttm_bo_vm.c @@ -33,6 +33,7 @@ #include <ttm/ttm_module.h> #include <ttm/ttm_bo_driver.h> #include <ttm/ttm_placement.h> +#include <drm/drm_vma_manager.h> #include <linux/mm.h> #include <linux/rbtree.h> #include <linux/module.h> @@ -40,37 +41,6 @@
#define TTM_BO_VM_NUM_PREFAULT 16
-static struct ttm_buffer_object *ttm_bo_vm_lookup_rb(struct ttm_bo_device *bdev, - unsigned long page_start, - unsigned long num_pages) -{ - struct rb_node *cur = bdev->addr_space_rb.rb_node; - unsigned long cur_offset; - struct ttm_buffer_object *bo; - struct ttm_buffer_object *best_bo = NULL; - - while (likely(cur != NULL)) { - bo = rb_entry(cur, struct ttm_buffer_object, vm_rb); - cur_offset = bo->vm_node->start; - if (page_start >= cur_offset) { - cur = cur->rb_right; - best_bo = bo; - if (page_start == cur_offset) - break; - } else - cur = cur->rb_left; - } - - if (unlikely(best_bo == NULL)) - return NULL; - - if (unlikely((best_bo->vm_node->start + best_bo->num_pages) < - (page_start + num_pages))) - return NULL; - - return best_bo; -} - static int ttm_bo_vm_fault(struct vm_area_struct *vma, struct vm_fault *vmf) { struct ttm_buffer_object *bo = (struct ttm_buffer_object *) @@ -146,9 +116,9 @@ static int ttm_bo_vm_fault(struct vm_area_struct *vma, struct vm_fault *vmf) }
page_offset = ((address - vma->vm_start) >> PAGE_SHIFT) + - bo->vm_node->start - vma->vm_pgoff; + drm_vma_node_start(&bo->vma_node) - vma->vm_pgoff; page_last = vma_pages(vma) + - bo->vm_node->start - vma->vm_pgoff; + drm_vma_node_start(&bo->vma_node) - vma->vm_pgoff;
if (unlikely(page_offset >= bo->num_pages)) { retval = VM_FAULT_SIGBUS; @@ -249,6 +219,30 @@ static const struct vm_operations_struct ttm_bo_vm_ops = { .close = ttm_bo_vm_close };
+static struct ttm_buffer_object *ttm_bo_vm_lookup(struct ttm_bo_device *bdev, + unsigned long offset, + unsigned long pages) +{ + struct drm_vma_offset_node *node; + struct ttm_buffer_object *bo = NULL; + + read_lock(&bdev->vm_lock); + + node = drm_vma_offset_lookup(&bdev->vma_manager, offset, pages); + if (likely(node)) { + bo = container_of(node, struct ttm_buffer_object, vma_node); + if (!kref_get_unless_zero(&bo->kref)) + bo = NULL; + } + + read_unlock(&bdev->vm_lock); + + if (!bo) + pr_err("Could not find buffer object to map\n"); + + return bo; +} + int ttm_bo_mmap(struct file *filp, struct vm_area_struct *vma, struct ttm_bo_device *bdev) { @@ -256,17 +250,9 @@ int ttm_bo_mmap(struct file *filp, struct vm_area_struct *vma, struct ttm_buffer_object *bo; int ret;
- read_lock(&bdev->vm_lock); - bo = ttm_bo_vm_lookup_rb(bdev, vma->vm_pgoff, - vma_pages(vma)); - if (likely(bo != NULL) && !kref_get_unless_zero(&bo->kref)) - bo = NULL; - read_unlock(&bdev->vm_lock); - - if (unlikely(bo == NULL)) { - pr_err("Could not find buffer object to map\n"); + bo = ttm_bo_vm_lookup(bdev, vma->vm_pgoff, vma_pages(vma)); + if (unlikely(!bo)) return -EINVAL; - }
driver = bo->bdev->driver; if (unlikely(!driver->verify_access)) { @@ -324,12 +310,7 @@ ssize_t ttm_bo_io(struct ttm_bo_device *bdev, struct file *filp, bool no_wait = false; bool dummy;
- read_lock(&bdev->vm_lock); - bo = ttm_bo_vm_lookup_rb(bdev, dev_offset, 1); - if (likely(bo != NULL)) - ttm_bo_reference(bo); - read_unlock(&bdev->vm_lock); - + bo = ttm_bo_vm_lookup(bdev, dev_offset, 1); if (unlikely(bo == NULL)) return -EFAULT;
@@ -343,7 +324,7 @@ ssize_t ttm_bo_io(struct ttm_bo_device *bdev, struct file *filp, if (unlikely(ret != 0)) goto out_unref;
- kmap_offset = dev_offset - bo->vm_node->start; + kmap_offset = dev_offset - drm_vma_node_start(&bo->vma_node); if (unlikely(kmap_offset >= bo->num_pages)) { ret = -EFBIG; goto out_unref; diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c b/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c index 7953d1f..0e67cf4 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c @@ -500,7 +500,7 @@ int vmw_dmabuf_alloc_ioctl(struct drm_device *dev, void *data, goto out_no_dmabuf;
rep->handle = handle; - rep->map_handle = dma_buf->base.addr_space_offset; + rep->map_handle = drm_vma_node_offset_addr(&dma_buf->base.vma_node); rep->cur_gmr_id = handle; rep->cur_gmr_offset = 0;
@@ -834,7 +834,7 @@ int vmw_dumb_map_offset(struct drm_file *file_priv, if (ret != 0) return -EINVAL;
- *offset = out_buf->base.addr_space_offset; + *offset = drm_vma_node_offset_addr(&out_buf->base.vma_node); vmw_dmabuf_unreference(&out_buf); return 0; } diff --git a/include/drm/ttm/ttm_bo_api.h b/include/drm/ttm/ttm_bo_api.h index 8a6aa56..751eaff 100644 --- a/include/drm/ttm/ttm_bo_api.h +++ b/include/drm/ttm/ttm_bo_api.h @@ -32,12 +32,12 @@ #define _TTM_BO_API_H_
#include <drm/drm_hashtab.h> +#include <drm/drm_vma_manager.h> #include <linux/kref.h> #include <linux/list.h> #include <linux/wait.h> #include <linux/mutex.h> #include <linux/mm.h> -#include <linux/rbtree.h> #include <linux/bitmap.h> #include <linux/reservation.h>
@@ -145,7 +145,6 @@ struct ttm_tt; * @type: The bo type. * @destroy: Destruction function. If NULL, kfree is used. * @num_pages: Actual number of pages. - * @addr_space_offset: Address space offset. * @acc_size: Accounted size for this object. * @kref: Reference count of this buffer object. When this refcount reaches * zero, the object is put on the delayed delete list. @@ -166,8 +165,7 @@ struct ttm_tt; * @swap: List head for swap LRU list. * @sync_obj: Pointer to a synchronization object. * @priv_flags: Flags describing buffer object internal state. - * @vm_rb: Rb node for the vm rb tree. - * @vm_node: Address space manager node. + * @vma_node: Address space manager node. * @offset: The current GPU offset, which can have different meanings * depending on the memory type. For SYSTEM type memory, it should be 0. * @cur_placement: Hint of current placement. @@ -194,7 +192,6 @@ struct ttm_buffer_object { enum ttm_bo_type type; void (*destroy) (struct ttm_buffer_object *); unsigned long num_pages; - uint64_t addr_space_offset; size_t acc_size;
/** @@ -238,13 +235,7 @@ struct ttm_buffer_object { void *sync_obj; unsigned long priv_flags;
- /** - * Members protected by the bdev::vm_lock - */ - - struct rb_node vm_rb; - struct drm_mm_node *vm_node; - + struct drm_vma_offset_node vma_node;
/** * Special members that are protected by the reserve lock diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h index 984fc2d..d3b8479 100644 --- a/include/drm/ttm/ttm_bo_driver.h +++ b/include/drm/ttm/ttm_bo_driver.h @@ -36,6 +36,7 @@ #include <ttm/ttm_placement.h> #include <drm/drm_mm.h> #include <drm/drm_global.h> +#include <drm/drm_vma_manager.h> #include <linux/workqueue.h> #include <linux/fs.h> #include <linux/spinlock.h> @@ -519,7 +520,7 @@ struct ttm_bo_global { * @man: An array of mem_type_managers. * @fence_lock: Protects the synchronizing members on *all* bos belonging * to this device. - * @addr_space_mm: Range manager for the device address space. + * @vma_manager: Address space manager * lru_lock: Spinlock that protects the buffer+device lru lists and * ddestroy lists. * @val_seq: Current validation sequence. @@ -540,11 +541,11 @@ struct ttm_bo_device { rwlock_t vm_lock; struct ttm_mem_type_manager man[TTM_NUM_MEM_TYPES]; spinlock_t fence_lock; + /* * Protected by the vm lock. */ - struct rb_root addr_space_rb; - struct drm_mm addr_space_mm; + struct drm_vma_offset_manager vma_manager;
/* * Protected by the global:lru lock.
Instead of unmapping the nodes in TTM and GEM users manually, we provide a generic wrapper which does the correct thing for all vma-nodes.
v2: remove bdev->dev_mapping test in ttm_bo_unmap_virtual_unlocked() as ttm_mem_io_free_vm() does nothing in that case (io_reserved_vm is 0).
Reviewed-by: Daniel Vetter daniel.vetter@ffwll.ch Signed-off-by: David Herrmann dh.herrmann@gmail.com --- drivers/gpu/drm/i915/i915_gem.c | 6 +----- drivers/gpu/drm/ttm/ttm_bo.c | 11 +---------- include/drm/drm_vma_manager.h | 16 ++++++++++++++++ 3 files changed, 18 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index ca1ceb0..8ab41a5 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -1426,11 +1426,7 @@ i915_gem_release_mmap(struct drm_i915_gem_object *obj) if (!obj->fault_mappable) return;
- if (obj->base.dev->dev_mapping) - unmap_mapping_range(obj->base.dev->dev_mapping, - (loff_t)drm_vma_node_offset_addr(&obj->base.vma_node), - obj->base.size, 1); - + drm_vma_node_unmap(&obj->base.vma_node, obj->base.dev->dev_mapping); obj->fault_mappable = false; }
diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c index 245850b..3fec6f99 100644 --- a/drivers/gpu/drm/ttm/ttm_bo.c +++ b/drivers/gpu/drm/ttm/ttm_bo.c @@ -1493,17 +1493,8 @@ bool ttm_mem_reg_is_pci(struct ttm_bo_device *bdev, struct ttm_mem_reg *mem) void ttm_bo_unmap_virtual_locked(struct ttm_buffer_object *bo) { struct ttm_bo_device *bdev = bo->bdev; - loff_t offset, holelen;
- if (!bdev->dev_mapping) - return; - - if (drm_vma_node_has_offset(&bo->vma_node)) { - offset = (loff_t) drm_vma_node_offset_addr(&bo->vma_node); - holelen = ((loff_t) bo->mem.num_pages) << PAGE_SHIFT; - - unmap_mapping_range(bdev->dev_mapping, offset, holelen, 1); - } + drm_vma_node_unmap(&bo->vma_node, bdev->dev_mapping); ttm_mem_io_free_vm(bo); }
diff --git a/include/drm/drm_vma_manager.h b/include/drm/drm_vma_manager.h index 4fcc441..355a0e5 100644 --- a/include/drm/drm_vma_manager.h +++ b/include/drm/drm_vma_manager.h @@ -24,6 +24,7 @@ */
#include <drm/drm_mm.h> +#include <linux/mm.h> #include <linux/module.h> #include <linux/rbtree.h> #include <linux/spinlock.h> @@ -135,4 +136,19 @@ static inline __u64 drm_vma_node_offset_addr(struct drm_vma_offset_node *node) return ((__u64)node->vm_node.start) << PAGE_SHIFT; }
+/** drm_vma_node_unmap() + * + * Unmap all userspace mappings for a given offset node. The mappings must be + * associated with the @file_mapping address-space. If no offset exists or + * the address-space is invalid, nothing is done. + */ +static inline void drm_vma_node_unmap(struct drm_vma_offset_node *node, + struct address_space *file_mapping) +{ + if (file_mapping && drm_vma_node_has_offset(node)) + unmap_mapping_range(file_mapping, + drm_vma_node_offset_addr(node), + node->vm_pages << PAGE_SHIFT, 1); +} + #endif /* __DRM_VMA_MANAGER_H__ */
Add a "best_match" argument similar to the drm_mm_search_*() helpers so we can convert TTM to use them in follow up patches.
Also move the non-generic aliases to drm_mm.h header instead of keeping them in the source file.
Signed-off-by: David Herrmann dh.herrmann@gmail.com --- drivers/gpu/drm/drm_mm.c | 23 ++++------------------- drivers/gpu/drm/drm_vma_manager.c | 4 ++-- drivers/gpu/drm/i915/i915_gem.c | 3 ++- drivers/gpu/drm/sis/sis_mm.c | 4 ++-- drivers/gpu/drm/via/via_mm.c | 4 ++-- include/drm/drm_mm.h | 38 ++++++++++++++++++++++++++------------ 6 files changed, 38 insertions(+), 38 deletions(-)
diff --git a/drivers/gpu/drm/drm_mm.c b/drivers/gpu/drm/drm_mm.c index 543b9b3..068abf4 100644 --- a/drivers/gpu/drm/drm_mm.c +++ b/drivers/gpu/drm/drm_mm.c @@ -217,12 +217,12 @@ EXPORT_SYMBOL(drm_mm_get_block_generic); */ int drm_mm_insert_node_generic(struct drm_mm *mm, struct drm_mm_node *node, unsigned long size, unsigned alignment, - unsigned long color) + unsigned long color, bool best_match) { struct drm_mm_node *hole_node;
hole_node = drm_mm_search_free_generic(mm, size, alignment, - color, 0); + color, best_match); if (!hole_node) return -ENOSPC;
@@ -231,13 +231,6 @@ int drm_mm_insert_node_generic(struct drm_mm *mm, struct drm_mm_node *node, } EXPORT_SYMBOL(drm_mm_insert_node_generic);
-int drm_mm_insert_node(struct drm_mm *mm, struct drm_mm_node *node, - unsigned long size, unsigned alignment) -{ - return drm_mm_insert_node_generic(mm, node, size, alignment, 0); -} -EXPORT_SYMBOL(drm_mm_insert_node); - static void drm_mm_insert_helper_range(struct drm_mm_node *hole_node, struct drm_mm_node *node, unsigned long size, unsigned alignment, @@ -318,13 +311,13 @@ EXPORT_SYMBOL(drm_mm_get_block_range_generic); */ int drm_mm_insert_node_in_range_generic(struct drm_mm *mm, struct drm_mm_node *node, unsigned long size, unsigned alignment, unsigned long color, - unsigned long start, unsigned long end) + unsigned long start, unsigned long end, bool best_match) { struct drm_mm_node *hole_node;
hole_node = drm_mm_search_free_in_range_generic(mm, size, alignment, color, - start, end, 0); + start, end, best_match); if (!hole_node) return -ENOSPC;
@@ -335,14 +328,6 @@ int drm_mm_insert_node_in_range_generic(struct drm_mm *mm, struct drm_mm_node *n } EXPORT_SYMBOL(drm_mm_insert_node_in_range_generic);
-int drm_mm_insert_node_in_range(struct drm_mm *mm, struct drm_mm_node *node, - unsigned long size, unsigned alignment, - unsigned long start, unsigned long end) -{ - return drm_mm_insert_node_in_range_generic(mm, node, size, alignment, 0, start, end); -} -EXPORT_SYMBOL(drm_mm_insert_node_in_range); - /** * Remove a memory node from the allocator. */ diff --git a/drivers/gpu/drm/drm_vma_manager.c b/drivers/gpu/drm/drm_vma_manager.c index 4b5f415..d07247e 100644 --- a/drivers/gpu/drm/drm_vma_manager.c +++ b/drivers/gpu/drm/drm_vma_manager.c @@ -218,8 +218,8 @@ int drm_vma_offset_add(struct drm_vma_offset_manager *mgr, goto out_unlock; }
- ret = drm_mm_insert_node_generic(&mgr->vm_addr_space_mm, - &node->vm_node, pages, 0, 0); + ret = drm_mm_insert_node(&mgr->vm_addr_space_mm, &node->vm_node, + pages, 0, false); if (ret) goto out_unlock;
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 8ab41a5..324f3c5 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -3130,7 +3130,8 @@ i915_gem_object_bind_to_gtt(struct drm_i915_gem_object *obj, search_free: ret = drm_mm_insert_node_in_range_generic(&dev_priv->mm.gtt_space, node, size, alignment, - obj->cache_level, 0, gtt_max); + obj->cache_level, 0, gtt_max, + false); if (ret) { ret = i915_gem_evict_something(dev, size, alignment, obj->cache_level, diff --git a/drivers/gpu/drm/sis/sis_mm.c b/drivers/gpu/drm/sis/sis_mm.c index 9a43d98..85da1a4 100644 --- a/drivers/gpu/drm/sis/sis_mm.c +++ b/drivers/gpu/drm/sis/sis_mm.c @@ -109,7 +109,7 @@ static int sis_drm_alloc(struct drm_device *dev, struct drm_file *file, if (pool == AGP_TYPE) { retval = drm_mm_insert_node(&dev_priv->agp_mm, &item->mm_node, - mem->size, 0); + mem->size, 0, false); offset = item->mm_node.start; } else { #if defined(CONFIG_FB_SIS) || defined(CONFIG_FB_SIS_MODULE) @@ -121,7 +121,7 @@ static int sis_drm_alloc(struct drm_device *dev, struct drm_file *file, #else retval = drm_mm_insert_node(&dev_priv->vram_mm, &item->mm_node, - mem->size, 0); + mem->size, 0, false); offset = item->mm_node.start; #endif } diff --git a/drivers/gpu/drm/via/via_mm.c b/drivers/gpu/drm/via/via_mm.c index 0ab93ff..10b962e 100644 --- a/drivers/gpu/drm/via/via_mm.c +++ b/drivers/gpu/drm/via/via_mm.c @@ -140,11 +140,11 @@ int via_mem_alloc(struct drm_device *dev, void *data, if (mem->type == VIA_MEM_AGP) retval = drm_mm_insert_node(&dev_priv->agp_mm, &item->mm_node, - tmpSize, 0); + tmpSize, 0, false); else retval = drm_mm_insert_node(&dev_priv->vram_mm, &item->mm_node, - tmpSize, 0); + tmpSize, 0, false); if (retval) goto fail_alloc;
diff --git a/include/drm/drm_mm.h b/include/drm/drm_mm.h index 4d06edb..adae5db 100644 --- a/include/drm/drm_mm.h +++ b/include/drm/drm_mm.h @@ -188,28 +188,42 @@ static inline struct drm_mm_node *drm_mm_get_block_atomic_range( start, end, 1); }
-extern int drm_mm_insert_node(struct drm_mm *mm, - struct drm_mm_node *node, - unsigned long size, - unsigned alignment); -extern int drm_mm_insert_node_in_range(struct drm_mm *mm, - struct drm_mm_node *node, - unsigned long size, - unsigned alignment, - unsigned long start, - unsigned long end); extern int drm_mm_insert_node_generic(struct drm_mm *mm, struct drm_mm_node *node, unsigned long size, unsigned alignment, - unsigned long color); + unsigned long color, + bool best_match); +static inline int drm_mm_insert_node(struct drm_mm *mm, + struct drm_mm_node *node, + unsigned long size, + unsigned alignment, + bool best_match) +{ + return drm_mm_insert_node_generic(mm, node, size, alignment, 0, + best_match); +} + extern int drm_mm_insert_node_in_range_generic(struct drm_mm *mm, struct drm_mm_node *node, unsigned long size, unsigned alignment, unsigned long color, unsigned long start, - unsigned long end); + unsigned long end, + bool best_match); +static inline int drm_mm_insert_node_in_range(struct drm_mm *mm, + struct drm_mm_node *node, + unsigned long size, + unsigned alignment, + unsigned long start, + unsigned long end, + bool best_match) +{ + return drm_mm_insert_node_in_range_generic(mm, node, size, alignment, + 0, start, end, best_match); +} + extern void drm_mm_put_block(struct drm_mm_node *cur); extern void drm_mm_remove_node(struct drm_mm_node *node); extern void drm_mm_replace_node(struct drm_mm_node *old, struct drm_mm_node *new);
Instead of calling drm_mm_pre_get() in a row, we now preallocate the node and then use the atomic insertion functions. This has the exact same semantics and there is no reason to use the racy pre-allocations.
Signed-off-by: David Herrmann dh.herrmann@gmail.com --- drivers/gpu/drm/ttm/ttm_bo_manager.c | 40 +++++++++++++++++------------------- 1 file changed, 19 insertions(+), 21 deletions(-)
diff --git a/drivers/gpu/drm/ttm/ttm_bo_manager.c b/drivers/gpu/drm/ttm/ttm_bo_manager.c index e4367f9..cbd2ec7 100644 --- a/drivers/gpu/drm/ttm/ttm_bo_manager.c +++ b/drivers/gpu/drm/ttm/ttm_bo_manager.c @@ -61,28 +61,24 @@ static int ttm_bo_man_get_node(struct ttm_mem_type_manager *man, lpfn = placement->lpfn; if (!lpfn) lpfn = man->size; - do { - ret = drm_mm_pre_get(mm); - if (unlikely(ret)) - return ret;
- spin_lock(&rman->lock); - node = drm_mm_search_free_in_range(mm, - mem->num_pages, mem->page_alignment, - placement->fpfn, lpfn, 1); - if (unlikely(node == NULL)) { - spin_unlock(&rman->lock); - return 0; - } - node = drm_mm_get_block_atomic_range(node, mem->num_pages, - mem->page_alignment, - placement->fpfn, - lpfn); - spin_unlock(&rman->lock); - } while (node == NULL); + node = kzalloc(sizeof(*node), GFP_KERNEL); + if (!node) + return -ENOMEM; + + spin_lock(&rman->lock); + ret = drm_mm_insert_node_in_range(mm, node, mem->num_pages, + mem->page_alignment, + placement->fpfn, lpfn, true); + spin_unlock(&rman->lock); + + if (unlikely(ret)) { + kfree(node); + } else { + mem->mm_node = node; + mem->start = node->start; + }
- mem->mm_node = node; - mem->start = node->start; return 0; }
@@ -93,8 +89,10 @@ static void ttm_bo_man_put_node(struct ttm_mem_type_manager *man,
if (mem->mm_node) { spin_lock(&rman->lock); - drm_mm_put_block(mem->mm_node); + drm_mm_remove_node(mem->mm_node); spin_unlock(&rman->lock); + + kfree(mem->mm_node); mem->mm_node = NULL; } }
i915 is the last user of the weird search+get_block drm_mm API. Convert it to an explicit kmalloc()+insert_node().
Signed-off-by: David Herrmann dh.herrmann@gmail.com --- drivers/gpu/drm/i915/i915_gem.c | 9 +++-- drivers/gpu/drm/i915/i915_gem_stolen.c | 72 ++++++++++++++++++++++------------ 2 files changed, 53 insertions(+), 28 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 324f3c5..f7c3b09 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -2628,7 +2628,8 @@ i915_gem_object_unbind(struct drm_i915_gem_object *obj) /* Avoid an unnecessary call to unbind on rebind. */ obj->map_and_fenceable = true;
- drm_mm_put_block(obj->gtt_space); + drm_mm_remove_node(obj->gtt_space); + kfree(obj->gtt_space); obj->gtt_space = NULL; obj->gtt_offset = 0;
@@ -3146,14 +3147,16 @@ search_free: } if (WARN_ON(!i915_gem_valid_gtt_space(dev, node, obj->cache_level))) { i915_gem_object_unpin_pages(obj); - drm_mm_put_block(node); + drm_mm_remove_node(node); + kfree(node); return -EINVAL; }
ret = i915_gem_gtt_prepare_object(obj); if (ret) { i915_gem_object_unpin_pages(obj); - drm_mm_put_block(node); + drm_mm_remove_node(node); + kfree(node); return ret; }
diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c index 982d473..820e78f 100644 --- a/drivers/gpu/drm/i915/i915_gem_stolen.c +++ b/drivers/gpu/drm/i915/i915_gem_stolen.c @@ -95,31 +95,39 @@ static int i915_setup_compression(struct drm_device *dev, int size) { struct drm_i915_private *dev_priv = dev->dev_private; struct drm_mm_node *compressed_fb, *uninitialized_var(compressed_llb); + int ret;
- /* Try to over-allocate to reduce reallocations and fragmentation */ - compressed_fb = drm_mm_search_free(&dev_priv->mm.stolen, - size <<= 1, 4096, 0); - if (!compressed_fb) - compressed_fb = drm_mm_search_free(&dev_priv->mm.stolen, - size >>= 1, 4096, 0); - if (compressed_fb) - compressed_fb = drm_mm_get_block(compressed_fb, size, 4096); + compressed_fb = kzalloc(sizeof(*compressed_fb), GFP_KERNEL); if (!compressed_fb) goto err;
+ /* Try to over-allocate to reduce reallocations and fragmentation */ + ret = drm_mm_insert_node(&dev_priv->mm.stolen, compressed_fb, + size <<= 1, 4096, false); + if (ret) + ret = drm_mm_insert_node(&dev_priv->mm.stolen, compressed_fb, + size >>= 1, 4096, false); + if (ret) { + kfree(compressed_fb); + goto err; + } + if (HAS_PCH_SPLIT(dev)) I915_WRITE(ILK_DPFC_CB_BASE, compressed_fb->start); else if (IS_GM45(dev)) { I915_WRITE(DPFC_CB_BASE, compressed_fb->start); } else { - compressed_llb = drm_mm_search_free(&dev_priv->mm.stolen, - 4096, 4096, 0); - if (compressed_llb) - compressed_llb = drm_mm_get_block(compressed_llb, - 4096, 4096); + compressed_llb = kzalloc(sizeof(*compressed_llb), GFP_KERNEL); if (!compressed_llb) goto err_fb;
+ ret = drm_mm_insert_node(&dev_priv->mm.stolen, compressed_llb, + 4096, 4096, false); + if (ret) { + kfree(compressed_llb); + goto err_fb; + } + dev_priv->compressed_llb = compressed_llb;
I915_WRITE(FBC_CFB_BASE, @@ -137,7 +145,8 @@ static int i915_setup_compression(struct drm_device *dev, int size) return 0;
err_fb: - drm_mm_put_block(compressed_fb); + drm_mm_remove_node(compressed_fb); + kfree(compressed_fb); err: pr_info_once("drm: not enough stolen space for compressed buffer (need %d more bytes), disabling. Hint: you may be able to increase stolen memory size in the BIOS to avoid this.\n", size); return -ENOSPC; @@ -166,11 +175,15 @@ void i915_gem_stolen_cleanup_compression(struct drm_device *dev) if (dev_priv->cfb_size == 0) return;
- if (dev_priv->compressed_fb) - drm_mm_put_block(dev_priv->compressed_fb); + if (dev_priv->compressed_fb) { + drm_mm_remove_node(dev_priv->compressed_fb); + kfree(dev_priv->compressed_fb); + }
- if (dev_priv->compressed_llb) - drm_mm_put_block(dev_priv->compressed_llb); + if (dev_priv->compressed_llb) { + drm_mm_remove_node(dev_priv->compressed_llb); + kfree(dev_priv->compressed_llb); + }
dev_priv->cfb_size = 0; } @@ -302,6 +315,7 @@ i915_gem_object_create_stolen(struct drm_device *dev, u32 size) struct drm_i915_private *dev_priv = dev->dev_private; struct drm_i915_gem_object *obj; struct drm_mm_node *stolen; + int ret;
if (!drm_mm_initialized(&dev_priv->mm.stolen)) return NULL; @@ -310,17 +324,23 @@ i915_gem_object_create_stolen(struct drm_device *dev, u32 size) if (size == 0) return NULL;
- stolen = drm_mm_search_free(&dev_priv->mm.stolen, size, 4096, 0); - if (stolen) - stolen = drm_mm_get_block(stolen, size, 4096); - if (stolen == NULL) + stolen = kzalloc(sizeof(*stolen), GFP_KERNEL); + if (!stolen) return NULL;
+ ret = drm_mm_insert_node(&dev_priv->mm.stolen, stolen, size, + 4096, false); + if (ret) { + kfree(stolen); + return NULL; + } + obj = _i915_gem_object_create_stolen(dev, stolen); if (obj) return obj;
- drm_mm_put_block(stolen); + drm_mm_remove_node(stolen); + kfree(stolen); return NULL; }
@@ -358,7 +378,8 @@ i915_gem_object_create_stolen_for_preallocated(struct drm_device *dev, obj = _i915_gem_object_create_stolen(dev, stolen); if (obj == NULL) { DRM_DEBUG_KMS("failed to allocate stolen object\n"); - drm_mm_put_block(stolen); + drm_mm_remove_node(stolen); + kfree(stolen); return NULL; }
@@ -396,7 +417,8 @@ void i915_gem_object_release_stolen(struct drm_i915_gem_object *obj) { if (obj->stolen) { - drm_mm_put_block(obj->stolen); + drm_mm_remove_node(obj->stolen); + kfree(obj->stolen); obj->stolen = NULL; } }
These calls are no longer used. Remove them.
Signed-off-by: David Herrmann dh.herrmann@gmail.com --- drivers/gpu/drm/drm_mm.c | 116 ++++++++--------------------------------------- include/drm/drm_mm.h | 74 ------------------------------ 2 files changed, 19 insertions(+), 171 deletions(-)
diff --git a/drivers/gpu/drm/drm_mm.c b/drivers/gpu/drm/drm_mm.c index 068abf4..fd3e9f9 100644 --- a/drivers/gpu/drm/drm_mm.c +++ b/drivers/gpu/drm/drm_mm.c @@ -49,6 +49,19 @@
#define MM_UNUSED_TARGET 4
+static struct drm_mm_node *drm_mm_search_free_generic(const struct drm_mm *mm, + unsigned long size, + unsigned alignment, + unsigned long color, + bool best_match); +static struct drm_mm_node *drm_mm_search_free_in_range_generic(const struct drm_mm *mm, + unsigned long size, + unsigned alignment, + unsigned long color, + unsigned long start, + unsigned long end, + bool best_match); + static struct drm_mm_node *drm_mm_kmalloc(struct drm_mm *mm, int atomic) { struct drm_mm_node *child; @@ -74,34 +87,6 @@ static struct drm_mm_node *drm_mm_kmalloc(struct drm_mm *mm, int atomic) return child; }
-/* drm_mm_pre_get() - pre allocate drm_mm_node structure - * drm_mm: memory manager struct we are pre-allocating for - * - * Returns 0 on success or -ENOMEM if allocation fails. - */ -int drm_mm_pre_get(struct drm_mm *mm) -{ - struct drm_mm_node *node; - - spin_lock(&mm->unused_lock); - while (mm->num_unused < MM_UNUSED_TARGET) { - spin_unlock(&mm->unused_lock); - node = kzalloc(sizeof(*node), GFP_KERNEL); - spin_lock(&mm->unused_lock); - - if (unlikely(node == NULL)) { - int ret = (mm->num_unused < 2) ? -ENOMEM : 0; - spin_unlock(&mm->unused_lock); - return ret; - } - ++mm->num_unused; - list_add_tail(&node->node_list, &mm->unused_nodes); - } - spin_unlock(&mm->unused_lock); - return 0; -} -EXPORT_SYMBOL(drm_mm_pre_get); - static void drm_mm_insert_helper(struct drm_mm_node *hole_node, struct drm_mm_node *node, unsigned long size, unsigned alignment, @@ -192,24 +177,6 @@ struct drm_mm_node *drm_mm_create_block(struct drm_mm *mm, } EXPORT_SYMBOL(drm_mm_create_block);
-struct drm_mm_node *drm_mm_get_block_generic(struct drm_mm_node *hole_node, - unsigned long size, - unsigned alignment, - unsigned long color, - int atomic) -{ - struct drm_mm_node *node; - - node = drm_mm_kmalloc(hole_node->mm, atomic); - if (unlikely(node == NULL)) - return NULL; - - drm_mm_insert_helper(hole_node, node, size, alignment, color); - - return node; -} -EXPORT_SYMBOL(drm_mm_get_block_generic); - /** * Search for free space and insert a preallocated memory node. Returns * -ENOSPC if no suitable free area is available. The preallocated memory node @@ -283,27 +250,6 @@ static void drm_mm_insert_helper_range(struct drm_mm_node *hole_node, } }
-struct drm_mm_node *drm_mm_get_block_range_generic(struct drm_mm_node *hole_node, - unsigned long size, - unsigned alignment, - unsigned long color, - unsigned long start, - unsigned long end, - int atomic) -{ - struct drm_mm_node *node; - - node = drm_mm_kmalloc(hole_node->mm, atomic); - if (unlikely(node == NULL)) - return NULL; - - drm_mm_insert_helper_range(hole_node, node, size, alignment, color, - start, end); - - return node; -} -EXPORT_SYMBOL(drm_mm_get_block_range_generic); - /** * Search for free space and insert a preallocated memory node. Returns * -ENOSPC if no suitable free area is available. This is for range @@ -362,28 +308,6 @@ void drm_mm_remove_node(struct drm_mm_node *node) } EXPORT_SYMBOL(drm_mm_remove_node);
-/* - * Remove a memory node from the allocator and free the allocated struct - * drm_mm_node. Only to be used on a struct drm_mm_node obtained by one of the - * drm_mm_get_block functions. - */ -void drm_mm_put_block(struct drm_mm_node *node) -{ - - struct drm_mm *mm = node->mm; - - drm_mm_remove_node(node); - - spin_lock(&mm->unused_lock); - if (mm->num_unused < MM_UNUSED_TARGET) { - list_add(&node->node_list, &mm->unused_nodes); - ++mm->num_unused; - } else - kfree(node); - spin_unlock(&mm->unused_lock); -} -EXPORT_SYMBOL(drm_mm_put_block); - static int check_free_hole(unsigned long start, unsigned long end, unsigned long size, unsigned alignment) { @@ -399,11 +323,11 @@ static int check_free_hole(unsigned long start, unsigned long end, return end >= start + size; }
-struct drm_mm_node *drm_mm_search_free_generic(const struct drm_mm *mm, - unsigned long size, - unsigned alignment, - unsigned long color, - bool best_match) +static struct drm_mm_node *drm_mm_search_free_generic(const struct drm_mm *mm, + unsigned long size, + unsigned alignment, + unsigned long color, + bool best_match) { struct drm_mm_node *entry; struct drm_mm_node *best; @@ -437,9 +361,8 @@ struct drm_mm_node *drm_mm_search_free_generic(const struct drm_mm *mm,
return best; } -EXPORT_SYMBOL(drm_mm_search_free_generic);
-struct drm_mm_node *drm_mm_search_free_in_range_generic(const struct drm_mm *mm, +static struct drm_mm_node *drm_mm_search_free_in_range_generic(const struct drm_mm *mm, unsigned long size, unsigned alignment, unsigned long color, @@ -484,7 +407,6 @@ struct drm_mm_node *drm_mm_search_free_in_range_generic(const struct drm_mm *mm,
return best; } -EXPORT_SYMBOL(drm_mm_search_free_in_range_generic);
/** * Moves an allocation. To be used with embedded struct drm_mm_node. diff --git a/include/drm/drm_mm.h b/include/drm/drm_mm.h index adae5db..b96c58d 100644 --- a/include/drm/drm_mm.h +++ b/include/drm/drm_mm.h @@ -147,46 +147,6 @@ extern struct drm_mm_node *drm_mm_get_block_generic(struct drm_mm_node *node, unsigned alignment, unsigned long color, int atomic); -extern struct drm_mm_node *drm_mm_get_block_range_generic( - struct drm_mm_node *node, - unsigned long size, - unsigned alignment, - unsigned long color, - unsigned long start, - unsigned long end, - int atomic); -static inline struct drm_mm_node *drm_mm_get_block(struct drm_mm_node *parent, - unsigned long size, - unsigned alignment) -{ - return drm_mm_get_block_generic(parent, size, alignment, 0, 0); -} -static inline struct drm_mm_node *drm_mm_get_block_atomic(struct drm_mm_node *parent, - unsigned long size, - unsigned alignment) -{ - return drm_mm_get_block_generic(parent, size, alignment, 0, 1); -} -static inline struct drm_mm_node *drm_mm_get_block_range( - struct drm_mm_node *parent, - unsigned long size, - unsigned alignment, - unsigned long start, - unsigned long end) -{ - return drm_mm_get_block_range_generic(parent, size, alignment, 0, - start, end, 0); -} -static inline struct drm_mm_node *drm_mm_get_block_atomic_range( - struct drm_mm_node *parent, - unsigned long size, - unsigned alignment, - unsigned long start, - unsigned long end) -{ - return drm_mm_get_block_range_generic(parent, size, alignment, 0, - start, end, 1); -}
extern int drm_mm_insert_node_generic(struct drm_mm *mm, struct drm_mm_node *node, @@ -224,47 +184,13 @@ static inline int drm_mm_insert_node_in_range(struct drm_mm *mm, 0, start, end, best_match); }
-extern void drm_mm_put_block(struct drm_mm_node *cur); extern void drm_mm_remove_node(struct drm_mm_node *node); extern void drm_mm_replace_node(struct drm_mm_node *old, struct drm_mm_node *new); -extern struct drm_mm_node *drm_mm_search_free_generic(const struct drm_mm *mm, - unsigned long size, - unsigned alignment, - unsigned long color, - bool best_match); -extern struct drm_mm_node *drm_mm_search_free_in_range_generic( - const struct drm_mm *mm, - unsigned long size, - unsigned alignment, - unsigned long color, - unsigned long start, - unsigned long end, - bool best_match); -static inline struct drm_mm_node *drm_mm_search_free(const struct drm_mm *mm, - unsigned long size, - unsigned alignment, - bool best_match) -{ - return drm_mm_search_free_generic(mm,size, alignment, 0, best_match); -} -static inline struct drm_mm_node *drm_mm_search_free_in_range( - const struct drm_mm *mm, - unsigned long size, - unsigned alignment, - unsigned long start, - unsigned long end, - bool best_match) -{ - return drm_mm_search_free_in_range_generic(mm, size, alignment, 0, - start, end, best_match); -} - extern void drm_mm_init(struct drm_mm *mm, unsigned long start, unsigned long size); extern void drm_mm_takedown(struct drm_mm *mm); extern int drm_mm_clean(struct drm_mm *mm); -extern int drm_mm_pre_get(struct drm_mm *mm);
static inline struct drm_mm *drm_get_mm(struct drm_mm_node *block) {
The VMA offset manager uses a device-global address-space, hence, any user can currently map any offset-node they want. They only need to guess the right offset. If we wanted per open-file offset spaces, we'd either need VM_NONLINEAR mappings or multiple "struct address_space" trees. As both doesn't really scale, we implement access management in the VMA manager itself.
We use an rb-tree to store open-files for each VMA node. On each mmap call, GEM, TTM or the drivers must check whether the current user is allowed to map this file.
We add a separate lock for each node as there is no generic lock available for the caller to protect the node easily.
Signed-off-by: David Herrmann dh.herrmann@gmail.com --- drivers/gpu/drm/drm_vma_manager.c | 148 ++++++++++++++++++++++++++++++++++++++ include/drm/drm_vma_manager.h | 16 +++++ 2 files changed, 164 insertions(+)
diff --git a/drivers/gpu/drm/drm_vma_manager.c b/drivers/gpu/drm/drm_vma_manager.c index d07247e..781f7da 100644 --- a/drivers/gpu/drm/drm_vma_manager.c +++ b/drivers/gpu/drm/drm_vma_manager.c @@ -25,6 +25,7 @@ #include <drm/drmP.h> #include <drm/drm_mm.h> #include <drm/drm_vma_manager.h> +#include <linux/fs.h> #include <linux/mm.h> #include <linux/module.h> #include <linux/rbtree.h> @@ -58,6 +59,13 @@ * must always be page-aligned (as usual). * If you want to get a valid byte-based user-space address for a given offset, * please see drm_vma_node_offset_addr(). + * + * Additionally to offset management, the vma offset manager also handles access + * management. For every open-file context that is allowed to access a given + * node, you must call drm_vma_node_allow(). Otherwise, an mmap() call on this + * open-file with the offset of the node will fail with -EACCES. To revoke + * access again, use drm_vma_node_revoke(). However, the caller is responsible + * for destroying already existing mappings, if required. */
/** @@ -258,3 +266,143 @@ void drm_vma_offset_remove(struct drm_vma_offset_manager *mgr, write_unlock(&mgr->vm_lock); } EXPORT_SYMBOL(drm_vma_offset_remove); + +/** + * drm_vma_node_allow - Add open-file to list of allowed users + * @node: Node to modify + * @filp: Open file to add + * + * Add @filp to the list of allowed open-files for this node. If @filp is + * already on this list, the ref-count is incremented. + * + * The list of allowed-users is preserved across drm_vma_offset_add() and + * drm_vma_offset_remove() calls. You may even call it if the node is currently + * not added to any offset-manager. + * + * You must remove all open-files the same number of times as you added them + * before destroying the node. Otherwise, you will leak memory. + * + * This is locked against concurrent access internally. + * + * RETURNS: + * 0 on success, negative error code on internal failure (out-of-mem) + */ +int drm_vma_node_allow(struct drm_vma_offset_node *node, struct file *filp) +{ + struct rb_node **iter; + struct rb_node *parent = NULL; + struct drm_vma_offset_file *entry; + int ret = 0; + + write_lock(&node->vm_lock); + + iter = &node->vm_files.rb_node; + + while (likely(*iter)) { + parent = *iter; + entry = rb_entry(*iter, struct drm_vma_offset_file, vm_rb); + + if (filp == entry->vm_filp) { + entry->vm_count++; + goto unlock; + } else if (filp > entry->vm_filp) { + iter = &(*iter)->rb_right; + } else { + iter = &(*iter)->rb_left; + } + } + + entry = kmalloc(sizeof(*entry), GFP_KERNEL); + if (!entry) { + ret = -ENOMEM; + goto unlock; + } + + entry->vm_filp = filp; + entry->vm_count = 1; + rb_link_node(&entry->vm_rb, parent, iter); + rb_insert_color(&entry->vm_rb, &node->vm_files); + +unlock: + write_unlock(&node->vm_lock); + return ret; +} +EXPORT_SYMBOL(drm_vma_node_allow); + +/** + * drm_vma_node_revoke - Remove open-file from list of allowed users + * @node: Node to modify + * @filp: Open file to remove + * + * Decrement the ref-count of @filp in the list of allowed open-files on @node. + * If the ref-count drops to zero, remove @filp from the list. You must call + * this once for every drm_vma_node_allow() on @filp. + * + * This is locked against concurrent access internally. + * + * If @filp is not on the list, nothing is done. + */ +void drm_vma_node_revoke(struct drm_vma_offset_node *node, struct file *filp) +{ + struct drm_vma_offset_file *entry; + struct rb_node *iter; + + write_lock(&node->vm_lock); + + iter = node->vm_files.rb_node; + while (likely(iter)) { + entry = rb_entry(iter, struct drm_vma_offset_file, vm_rb); + if (filp == entry->vm_filp) { + if (!--entry->vm_count) { + rb_erase(&entry->vm_rb, &node->vm_files); + kfree(entry); + } + break; + } else if (filp > entry->vm_filp) { + iter = iter->rb_right; + } else { + iter = iter->rb_left; + } + } + + write_unlock(&node->vm_lock); +} +EXPORT_SYMBOL(drm_vma_node_revoke); + +/** + * drm_vma_node_is_allowed - Check whether an open-file is granted access + * @node: Node to check + * @filp: Open-file to check for + * + * Search the list in @node whether @filp is currently on the list of allowed + * open-files (see drm_vma_node_allow()). + * + * This is locked against concurrent access internally. + * + * RETURNS: + * true iff @filp is on the list + */ +bool drm_vma_node_is_allowed(struct drm_vma_offset_node *node, + struct file *filp) +{ + struct drm_vma_offset_file *entry; + struct rb_node *iter; + + read_lock(&node->vm_lock); + + iter = node->vm_files.rb_node; + while (likely(iter)) { + entry = rb_entry(iter, struct drm_vma_offset_file, vm_rb); + if (filp == entry->vm_filp) + break; + else if (filp > entry->vm_filp) + iter = iter->rb_right; + else + iter = iter->rb_left; + } + + read_unlock(&node->vm_lock); + + return iter; +} +EXPORT_SYMBOL(drm_vma_node_is_allowed); diff --git a/include/drm/drm_vma_manager.h b/include/drm/drm_vma_manager.h index 355a0e5..52bb189 100644 --- a/include/drm/drm_vma_manager.h +++ b/include/drm/drm_vma_manager.h @@ -24,16 +24,25 @@ */
#include <drm/drm_mm.h> +#include <linux/fs.h> #include <linux/mm.h> #include <linux/module.h> #include <linux/rbtree.h> #include <linux/spinlock.h> #include <linux/types.h>
+struct drm_vma_offset_file { + struct rb_node vm_rb; + struct file *vm_filp; + unsigned long vm_count; +}; + struct drm_vma_offset_node { + rwlock_t vm_lock; struct drm_mm_node vm_node; struct rb_node vm_rb; unsigned long vm_pages; + struct rb_root vm_files; };
struct drm_vma_offset_manager { @@ -54,6 +63,11 @@ int drm_vma_offset_add(struct drm_vma_offset_manager *mgr, void drm_vma_offset_remove(struct drm_vma_offset_manager *mgr, struct drm_vma_offset_node *node);
+int drm_vma_node_allow(struct drm_vma_offset_node *node, struct file *filp); +void drm_vma_node_revoke(struct drm_vma_offset_node *node, struct file *filp); +bool drm_vma_node_is_allowed(struct drm_vma_offset_node *node, + struct file *filp); + /** * drm_vma_offset_exact_lookup() - Look up node by exact address * @mgr: Manager object @@ -85,6 +99,8 @@ drm_vma_offset_exact_lookup(struct drm_vma_offset_manager *mgr, static inline void drm_vma_node_reset(struct drm_vma_offset_node *node) { memset(node, 0, sizeof(*node)); + node->vm_files = RB_ROOT; + rwlock_init(&node->vm_lock); }
/**
Correctly allow and revoke buffer access on each open/close via the new VMA offset manager.
Cc: Dave Airlie airlied@redhat.com Signed-off-by: David Herrmann dh.herrmann@gmail.com --- drivers/gpu/drm/ast/ast_drv.c | 2 ++ drivers/gpu/drm/ast/ast_drv.h | 4 ++++ drivers/gpu/drm/ast/ast_main.c | 15 +++++++++++++++ 3 files changed, 21 insertions(+)
diff --git a/drivers/gpu/drm/ast/ast_drv.c b/drivers/gpu/drm/ast/ast_drv.c index df0d0a0..1060f9c 100644 --- a/drivers/gpu/drm/ast/ast_drv.c +++ b/drivers/gpu/drm/ast/ast_drv.c @@ -214,6 +214,8 @@ static struct drm_driver driver = {
.gem_init_object = ast_gem_init_object, .gem_free_object = ast_gem_free_object, + .gem_open_object = ast_gem_open_object, + .gem_close_object = ast_gem_close_object, .dumb_create = ast_dumb_create, .dumb_map_offset = ast_dumb_mmap_offset, .dumb_destroy = ast_dumb_destroy, diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h index 622d4ae..f35946b 100644 --- a/drivers/gpu/drm/ast/ast_drv.h +++ b/drivers/gpu/drm/ast/ast_drv.h @@ -328,6 +328,10 @@ extern int ast_dumb_destroy(struct drm_file *file,
extern int ast_gem_init_object(struct drm_gem_object *obj); extern void ast_gem_free_object(struct drm_gem_object *obj); +extern int ast_gem_open_object(struct drm_gem_object *obj, + struct drm_file *file_priv); +extern void ast_gem_close_object(struct drm_gem_object *obj, + struct drm_file *file_priv); extern int ast_dumb_mmap_offset(struct drm_file *file, struct drm_device *dev, uint32_t handle, diff --git a/drivers/gpu/drm/ast/ast_main.c b/drivers/gpu/drm/ast/ast_main.c index c195dc2..f9efcf8 100644 --- a/drivers/gpu/drm/ast/ast_main.c +++ b/drivers/gpu/drm/ast/ast_main.c @@ -484,6 +484,21 @@ void ast_gem_free_object(struct drm_gem_object *obj) ast_bo_unref(&ast_bo); }
+int ast_gem_open_object(struct drm_gem_object *obj, + struct drm_file *file_priv) +{ + struct ast_bo *ast_bo = gem_to_ast_bo(obj); + + return drm_vma_node_allow(&ast_bo->bo.vma_node, file_priv->filp); +} + +void ast_gem_close_object(struct drm_gem_object *obj, + struct drm_file *file_priv) +{ + struct ast_bo *ast_bo = gem_to_ast_bo(obj); + + drm_vma_node_revoke(&ast_bo->bo.vma_node, file_priv->filp); +}
static inline u64 ast_bo_mmap_offset(struct ast_bo *bo) {
Correctly allow and revoke buffer access on each open/close via the new VMA offset manager.
Cc: Dave Airlie airlied@redhat.com Signed-off-by: David Herrmann dh.herrmann@gmail.com --- drivers/gpu/drm/cirrus/cirrus_drv.h | 4 ++++ drivers/gpu/drm/cirrus/cirrus_main.c | 15 +++++++++++++++ 2 files changed, 19 insertions(+)
diff --git a/drivers/gpu/drm/cirrus/cirrus_drv.h b/drivers/gpu/drm/cirrus/cirrus_drv.h index bae5560..6af655a 100644 --- a/drivers/gpu/drm/cirrus/cirrus_drv.h +++ b/drivers/gpu/drm/cirrus/cirrus_drv.h @@ -193,6 +193,10 @@ int cirrus_device_init(struct cirrus_device *cdev, void cirrus_device_fini(struct cirrus_device *cdev); int cirrus_gem_init_object(struct drm_gem_object *obj); void cirrus_gem_free_object(struct drm_gem_object *obj); +int cirrus_gem_open_object(struct drm_gem_object *obj, + struct drm_file *file_priv); +void cirrus_gem_close_object(struct drm_gem_object *obj, + struct drm_file *file_priv); int cirrus_dumb_mmap_offset(struct drm_file *file, struct drm_device *dev, uint32_t handle, diff --git a/drivers/gpu/drm/cirrus/cirrus_main.c b/drivers/gpu/drm/cirrus/cirrus_main.c index 3a7a0ef..345e77b 100644 --- a/drivers/gpu/drm/cirrus/cirrus_main.c +++ b/drivers/gpu/drm/cirrus/cirrus_main.c @@ -291,6 +291,21 @@ void cirrus_gem_free_object(struct drm_gem_object *obj) cirrus_bo_unref(&cirrus_bo); }
+int cirrus_gem_open_object(struct drm_gem_object *obj, + struct drm_file *file_priv) +{ + struct cirrus_bo *bo = gem_to_cirrus_bo(obj); + + return drm_vma_node_allow(&bo->bo.vma_node, file_priv->filp); +} + +void cirrus_gem_close_object(struct drm_gem_object *obj, + struct drm_file *file_priv) +{ + struct cirrus_bo *bo = gem_to_cirrus_bo(obj); + + drm_vma_node_revoke(&bo->bo.vma_node, file_priv->filp); +}
static inline u64 cirrus_bo_mmap_offset(struct cirrus_bo *bo) {
Correctly allow and revoke buffer access on each open/close via the new VMA offset manager.
Cc: Dave Airlie airlied@redhat.com Signed-off-by: David Herrmann dh.herrmann@gmail.com --- drivers/gpu/drm/mgag200/mgag200_drv.c | 2 ++ drivers/gpu/drm/mgag200/mgag200_drv.h | 4 ++++ drivers/gpu/drm/mgag200/mgag200_main.c | 15 +++++++++++++++ 3 files changed, 21 insertions(+)
diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.c b/drivers/gpu/drm/mgag200/mgag200_drv.c index 122b571..d26a244 100644 --- a/drivers/gpu/drm/mgag200/mgag200_drv.c +++ b/drivers/gpu/drm/mgag200/mgag200_drv.c @@ -102,6 +102,8 @@ static struct drm_driver driver = {
.gem_init_object = mgag200_gem_init_object, .gem_free_object = mgag200_gem_free_object, + .gem_open_object = mgag200_gem_open_object, + .gem_close_object = mgag200_gem_close_object, .dumb_create = mgag200_dumb_create, .dumb_map_offset = mgag200_dumb_mmap_offset, .dumb_destroy = mgag200_dumb_destroy, diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.h b/drivers/gpu/drm/mgag200/mgag200_drv.h index 12e2499..4e674be 100644 --- a/drivers/gpu/drm/mgag200/mgag200_drv.h +++ b/drivers/gpu/drm/mgag200/mgag200_drv.h @@ -268,6 +268,10 @@ int mgag200_dumb_destroy(struct drm_file *file, struct drm_device *dev, uint32_t handle); void mgag200_gem_free_object(struct drm_gem_object *obj); +int mgag200_gem_open_object(struct drm_gem_object *obj, + struct drm_file *file_priv); +void mgag200_gem_close_object(struct drm_gem_object *obj, + struct drm_file *file_priv); int mgag200_dumb_mmap_offset(struct drm_file *file, struct drm_device *dev, diff --git a/drivers/gpu/drm/mgag200/mgag200_main.c b/drivers/gpu/drm/mgag200/mgag200_main.c index 1a75ea3..26bee28 100644 --- a/drivers/gpu/drm/mgag200/mgag200_main.c +++ b/drivers/gpu/drm/mgag200/mgag200_main.c @@ -346,6 +346,21 @@ void mgag200_gem_free_object(struct drm_gem_object *obj) mgag200_bo_unref(&mgag200_bo); }
+int mgag200_gem_open_object(struct drm_gem_object *obj, + struct drm_file *file_priv) +{ + struct mgag200_bo *bo = gem_to_mga_bo(obj); + + return drm_vma_node_allow(&bo->bo.vma_node, file_priv->filp); +} + +void mgag200_gem_close_object(struct drm_gem_object *obj, + struct drm_file *file_priv) +{ + struct mgag200_bo *bo = gem_to_mga_bo(obj); + + drm_vma_node_revoke(&bo->bo.vma_node, file_priv->filp); +}
static inline u64 mgag200_bo_mmap_offset(struct mgag200_bo *bo) {
Correctly allow and revoke buffer access on each open/close via the new VMA offset manager.
Cc: Ben Skeggs bskeggs@redhat.com Signed-off-by: David Herrmann dh.herrmann@gmail.com --- drivers/gpu/drm/nouveau/nouveau_gem.c | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/nouveau_gem.c b/drivers/gpu/drm/nouveau/nouveau_gem.c index 86597eb..04d0a7d 100644 --- a/drivers/gpu/drm/nouveau/nouveau_gem.c +++ b/drivers/gpu/drm/nouveau/nouveau_gem.c @@ -73,32 +73,41 @@ nouveau_gem_object_open(struct drm_gem_object *gem, struct drm_file *file_priv) struct nouveau_vma *vma; int ret;
+ ret = drm_vma_node_allow(&nvbo->bo.vma_node, file_priv->filp); + if (ret) + return ret; + if (!cli->base.vm) return 0;
ret = ttm_bo_reserve(&nvbo->bo, false, false, false, 0); if (ret) - return ret; + goto err_node;
vma = nouveau_bo_vma_find(nvbo, cli->base.vm); if (!vma) { vma = kzalloc(sizeof(*vma), GFP_KERNEL); if (!vma) { ret = -ENOMEM; - goto out; + goto err_reserve; }
ret = nouveau_bo_vma_add(nvbo, cli->base.vm, vma); if (ret) { kfree(vma); - goto out; + goto err_reserve; } } else { vma->refcount++; }
-out: ttm_bo_unreserve(&nvbo->bo); + return 0; + +err_reserve: + ttm_bo_unreserve(&nvbo->bo); +err_node: + drm_vma_node_revoke(&nvbo->bo.vma_node, file_priv->filp); return ret; }
@@ -145,6 +154,8 @@ nouveau_gem_object_close(struct drm_gem_object *gem, struct drm_file *file_priv) struct nouveau_vma *vma; int ret;
+ drm_vma_node_revoke(&nvbo->bo.vma_node, file_priv->filp); + if (!cli->base.vm) return;
Correctly allow and revoke buffer access on each open/close via the new VMA offset manager.
Cc: Alex Deucher alexander.deucher@amd.com Signed-off-by: David Herrmann dh.herrmann@gmail.com --- drivers/gpu/drm/radeon/radeon_gem.c | 7 +++++++ 1 file changed, 7 insertions(+)
diff --git a/drivers/gpu/drm/radeon/radeon_gem.c b/drivers/gpu/drm/radeon/radeon_gem.c index aa79603..365819a 100644 --- a/drivers/gpu/drm/radeon/radeon_gem.c +++ b/drivers/gpu/drm/radeon/radeon_gem.c @@ -147,12 +147,17 @@ int radeon_gem_object_open(struct drm_gem_object *obj, struct drm_file *file_pri struct radeon_bo_va *bo_va; int r;
+ r = drm_vma_node_allow(&rbo->tbo.vma_node, file_priv->filp); + if (r) + return r; + if (rdev->family < CHIP_CAYMAN) { return 0; }
r = radeon_bo_reserve(rbo, false); if (r) { + drm_vma_node_revoke(&rbo->tbo.vma_node, file_priv->filp); return r; }
@@ -177,6 +182,8 @@ void radeon_gem_object_close(struct drm_gem_object *obj, struct radeon_bo_va *bo_va; int r;
+ drm_vma_node_revoke(&rbo->tbo.vma_node, file_priv->filp); + if (rdev->family < CHIP_CAYMAN) { return; }
Correctly allow and revoke buffer access on each open/close via the new VMA offset manager.
Cc: Dave Airlie airlied@redhat.com Signed-off-by: David Herrmann dh.herrmann@gmail.com --- drivers/gpu/drm/qxl/qxl_gem.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/qxl/qxl_gem.c b/drivers/gpu/drm/qxl/qxl_gem.c index a235693..fb38a5e 100644 --- a/drivers/gpu/drm/qxl/qxl_gem.c +++ b/drivers/gpu/drm/qxl/qxl_gem.c @@ -129,12 +129,17 @@ void qxl_gem_object_unpin(struct drm_gem_object *obj)
int qxl_gem_object_open(struct drm_gem_object *obj, struct drm_file *file_priv) { - return 0; + struct qxl_bo *qobj = obj->driver_private; + + return drm_vma_node_allow(&qobj->tbo.vma_node, file_priv->filp); }
void qxl_gem_object_close(struct drm_gem_object *obj, struct drm_file *file_priv) { + struct qxl_bo *qobj = obj->driver_private; + + drm_vma_node_revoke(&qobj->tbo.vma_node, file_priv->filp); }
int qxl_gem_init(struct qxl_device *qdev)
Correctly allow and revoke buffer access on each open/close via the new VMA offset manager.
Also remove unused vmw_user_dmabuf_reference() to prevent from using it later without correctly adding mmap permissions.
Cc: Thomas Hellstrom thellstrom@vmware.com Signed-off-by: David Herrmann dh.herrmann@gmail.com --- drivers/gpu/drm/vmwgfx/vmwgfx_resource.c | 29 +++++++++++++++++------------ 1 file changed, 17 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c b/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c index 0e67cf4..4d3f0ae 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c @@ -499,6 +499,12 @@ int vmw_dmabuf_alloc_ioctl(struct drm_device *dev, void *data, if (unlikely(ret != 0)) goto out_no_dmabuf;
+ ret = drm_vma_node_allow(&dma_buf->base.vma_node, file_priv->filp); + if (ret) { + vmw_dmabuf_unreference(&dma_buf); + goto out_no_dmabuf; + } + rep->handle = handle; rep->map_handle = drm_vma_node_offset_addr(&dma_buf->base.vma_node); rep->cur_gmr_id = handle; @@ -517,7 +523,18 @@ int vmw_dmabuf_unref_ioctl(struct drm_device *dev, void *data, { struct drm_vmw_unref_dmabuf_arg *arg = (struct drm_vmw_unref_dmabuf_arg *)data; + struct ttm_object_file *tfile = vmw_fpriv(file_priv)->tfile; + struct vmw_dma_buffer *dma_buf; + int ret; + + ret = vmw_user_dmabuf_lookup(tfile, arg->handle, &dma_buf); + if (ret) + return -EINVAL;
+ drm_vma_node_revoke(&dma_buf->base.vma_node, file_priv->filp); + vmw_dmabuf_unreference(&dma_buf); + + /* FIXME: is this equivalent to vmw_dmabuf_unreference(dma_buf)? */ return ttm_ref_object_base_unref(vmw_fpriv(file_priv)->tfile, arg->handle, TTM_REF_USAGE); @@ -551,18 +568,6 @@ int vmw_user_dmabuf_lookup(struct ttm_object_file *tfile, return 0; }
-int vmw_user_dmabuf_reference(struct ttm_object_file *tfile, - struct vmw_dma_buffer *dma_buf) -{ - struct vmw_user_dma_buffer *user_bo; - - if (dma_buf->base.destroy != vmw_user_dmabuf_destroy) - return -EINVAL; - - user_bo = container_of(dma_buf, struct vmw_user_dma_buffer, dma); - return ttm_ref_object_add(tfile, &user_bo->base, TTM_REF_USAGE, NULL); -} - /* * Stream management */
If a user does not have access to a given buffer, we must not allow them to mmap it. Otherwise, users could "guess" the buffer offsets of other users and get access to the buffer.
All TTM drivers already use the new VMA offset manager access management so we can enable TTM mmap access management now.
Signed-off-by: David Herrmann dh.herrmann@gmail.com --- drivers/gpu/drm/ttm/ttm_bo_vm.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/ttm/ttm_bo_vm.c b/drivers/gpu/drm/ttm/ttm_bo_vm.c index 54a67f1..756adc7 100644 --- a/drivers/gpu/drm/ttm/ttm_bo_vm.c +++ b/drivers/gpu/drm/ttm/ttm_bo_vm.c @@ -219,7 +219,8 @@ static const struct vm_operations_struct ttm_bo_vm_ops = { .close = ttm_bo_vm_close };
-static struct ttm_buffer_object *ttm_bo_vm_lookup(struct ttm_bo_device *bdev, +static struct ttm_buffer_object *ttm_bo_vm_lookup(struct file *filp, + struct ttm_bo_device *bdev, unsigned long offset, unsigned long pages) { @@ -229,7 +230,7 @@ static struct ttm_buffer_object *ttm_bo_vm_lookup(struct ttm_bo_device *bdev, read_lock(&bdev->vm_lock);
node = drm_vma_offset_lookup(&bdev->vma_manager, offset, pages); - if (likely(node)) { + if (likely(node) && drm_vma_node_is_allowed(node, filp)) { bo = container_of(node, struct ttm_buffer_object, vma_node); if (!kref_get_unless_zero(&bo->kref)) bo = NULL; @@ -250,7 +251,7 @@ int ttm_bo_mmap(struct file *filp, struct vm_area_struct *vma, struct ttm_buffer_object *bo; int ret;
- bo = ttm_bo_vm_lookup(bdev, vma->vm_pgoff, vma_pages(vma)); + bo = ttm_bo_vm_lookup(filp, bdev, vma->vm_pgoff, vma_pages(vma)); if (unlikely(!bo)) return -EINVAL;
@@ -310,7 +311,7 @@ ssize_t ttm_bo_io(struct ttm_bo_device *bdev, struct file *filp, bool no_wait = false; bool dummy;
- bo = ttm_bo_vm_lookup(bdev, dev_offset, 1); + bo = ttm_bo_vm_lookup(filp, bdev, dev_offset, 1); if (unlikely(bo == NULL)) return -EFAULT;
Implement automatic access management for mmap offsets for all GEM drivers. This prevents user-space applications from "guessing" GEM BO offsets and accessing buffers which they don't own.
Signed-off-by: David Herrmann dh.herrmann@gmail.com --- drivers/gpu/drm/drm_gem.c | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c index b5db89b..9d40ee3 100644 --- a/drivers/gpu/drm/drm_gem.c +++ b/drivers/gpu/drm/drm_gem.c @@ -240,6 +240,7 @@ drm_gem_handle_delete(struct drm_file *filp, u32 handle) spin_unlock(&filp->table_lock);
drm_gem_remove_prime_handles(obj, filp); + drm_vma_node_revoke(&obj->vma_node, filp->filp);
if (dev->driver->gem_close_object) dev->driver->gem_close_object(obj, filp); @@ -279,15 +280,23 @@ drm_gem_handle_create(struct drm_file *file_priv,
drm_gem_object_handle_reference(obj);
+ ret = drm_vma_node_allow(&obj->vma_node, file_priv->filp); + if (ret) + goto err_handle; + if (dev->driver->gem_open_object) { ret = dev->driver->gem_open_object(obj, file_priv); - if (ret) { - drm_gem_handle_delete(file_priv, *handlep); - return ret; - } + if (ret) + goto err_vma; }
return 0; + +err_vma: + drm_vma_node_revoke(&obj->vma_node, file_priv->filp); +err_handle: + drm_gem_handle_delete(file_priv, *handlep); + return ret; } EXPORT_SYMBOL(drm_gem_handle_create);
@@ -476,6 +485,7 @@ drm_gem_object_release_handle(int id, void *ptr, void *data) struct drm_device *dev = obj->dev;
drm_gem_remove_prime_handles(obj, file_priv); + drm_vma_node_revoke(&obj->vma_node, file_priv->filp);
if (dev->driver->gem_close_object) dev->driver->gem_close_object(obj, file_priv); @@ -668,6 +678,9 @@ int drm_gem_mmap(struct file *filp, struct vm_area_struct *vma) if (!node) { mutex_unlock(&dev->struct_mutex); return drm_mmap(filp, vma); + } else if (!drm_vma_node_is_allowed(node, filp)) { + mutex_unlock(&dev->struct_mutex); + return -EACCES; }
obj = container_of(node, struct drm_gem_object, vma_node);
Currently, both ranges overlap. Fix the limits so both ranges are mutually exclusive. Also use the occasion to convert whitespaces to tabs.
Cc: Martin Peres martin.peres@labri.fr Cc: Kristian Høgsberg krh@bitplanet.net Signed-off-by: David Herrmann dh.herrmann@gmail.com --- drivers/gpu/drm/drm_stub.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/drm_stub.c b/drivers/gpu/drm/drm_stub.c index 327ca19..d372e27 100644 --- a/drivers/gpu/drm/drm_stub.c +++ b/drivers/gpu/drm/drm_stub.c @@ -113,12 +113,12 @@ static int drm_minor_get_id(struct drm_device *dev, int type) int base = 0, limit = 63;
if (type == DRM_MINOR_CONTROL) { - base += 64; - limit = base + 127; - } else if (type == DRM_MINOR_RENDER) { - base += 128; - limit = base + 255; - } + base += 64; + limit = base + 63; + } else if (type == DRM_MINOR_RENDER) { + base += 128; + limit = base + 63; + }
mutex_lock(&dev->struct_mutex); ret = idr_alloc(&drm_minors_idr, NULL, base, limit, GFP_KERNEL);
Render nodes provide an API for userspace to use non-privileged GPU commands without any running DRM-Master. It is useful for offscreen rendering, GPGPU clients, and normal render clients which do not perform modesetting.
Compared to legacy clients, render clients no longer need any authentication to perform client ioctls. Instead, user-space controls render/client access to GPUs via filesystem access-modes on the render-node. Once a render-node was opened, a client has full access to the client/render operations on the GPU. However, no modesetting or ioctls that affect global state are allowed on render nodes.
To prevent privilege-escalation, drivers must explicitly state that they support render nodes. They must mark their render-only ioctls as DRM_RENDER_ALLOW so render clients can use them. Furthermore, they must support clients without any attached master.
If filesystem access-modes are not enough for fine-grained access control to render nodes (very unlikely, considering the versaitlity of FS-ACLs), you may still fall-back to fd-passing from server to client (which allows arbitrary access-control). However, note that revoking access is currently impossible and unlikely to get implemented.
Note: Render clients no longer have any associated DRM-Master as they are supposed to be independent of any server state. DRM core highly depends on file_priv->master to be non-NULL for modesetting/ctx/etc. commands. Therefore, drivers must be very careful to not require DRM-Master if they support DRIVER_RENDER.
Cc: Kristian Høgsberg krh@bitplanet.net Signed-off-by: David Herrmann dh.herrmann@gmail.com --- Documentation/DocBook/drm.tmpl | 71 ++++++++++++++++++++++++++++++++++++++++++ drivers/gpu/drm/drm_drv.c | 15 ++++----- drivers/gpu/drm/drm_fops.c | 14 ++++----- drivers/gpu/drm/drm_pci.c | 9 ++++++ drivers/gpu/drm/drm_platform.c | 9 ++++++ drivers/gpu/drm/drm_stub.c | 5 +++ drivers/gpu/drm/drm_usb.c | 9 ++++++ include/drm/drmP.h | 8 +++++ 8 files changed, 126 insertions(+), 14 deletions(-)
diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl index b02732d..44023b9 100644 --- a/Documentation/DocBook/drm.tmpl +++ b/Documentation/DocBook/drm.tmpl @@ -234,6 +234,12 @@ Driver implements DRM PRIME buffer sharing. </para></listitem> </varlistentry> + <varlistentry> + <term>DRIVER_RENDER</term> + <listitem><para> + Driver supports dedicated render nodes. + </para></listitem> + </varlistentry> </variablelist> </sect3> <sect3> @@ -2663,6 +2669,71 @@ int (*resume) (struct drm_device *);</synopsis> info, since man pages should cover the rest. </para>
+ <!-- External: render nodes --> + + <sect1> + <title>Render nodes</title> + <para> + DRM core provides multiple character-devices for user-space to use. + Depending on which device is opened, user-space can perform a different + set of operations (mainly ioctls). The primary node is always created + and called <term>card<num></term>. Additionally, a currently + unused control node, called <term>controlD<num></term> is also + created. The primary node provides all legacy operations and + historically was the only interface used by userspace. With KMS, the + control node was introduced. However, the planned KMS control interface + has never been written and so the control node stays unused to date. + </para> + <para> + With the increased use of offscreen renderers and GPGPU applications, + clients no longer require running compositors or graphics servers to + make use of a GPU. But the DRM API required unprivileged clients to + authenticate to a DRM-Master prior to getting GPU access. To avoid this + step and to grant clients GPU access without authenticating, render + nodes were introduced. Render nodes solely serve render clients, that + is, no modesetting or privileged ioctls can be issued on render nodes. + Only non-global rendering commands are allowed. If a driver supports + render nodes, it must advertise it via the <term>DRIVER_RENDER</term> + DRM driver capability. If not supported, the primary node must be used + for render clients together with the legacy drmAuth authentication + procedure. + </para> + <para> + If a driver advertises render node support, DRM core will create a + separate render node called <term>renderD<num></term>. There will + be one render node per device. No ioctls except for + <term>GEM_CLOSE</term>, <term>GEM_FLINK</term> and PRIME-related ioctls + will be allowed on this node. Especially <term>GEM_OPEN</term> will be + explicitly prohibited. Render nodes are designed to avoid the + buffer-leaks, which occur if clients guess the flink names or mmap + offsets on the legacy interface. Additionally to this basic interface, + drivers must mark their driver-dependent render-only ioctls as + <term>DRM_RENDER_ALLOW</term> so render clients can use them. Driver + authors must be careful not to allow any privileged ioctls on render + nodes. + </para> + <para> + With render nodes, user-space can now control access to the render node + via basic file-system access-modes. A running graphics server which + authenticates clients on the privileged primary/legacy node is no longer + required. Instead, a client can open the render node and is immediately + granted GPU access. Communication between clients (or servers) is done + via PRIME. FLINK from render node to legacy node is also supported, + however, FLINK from legacy node to render node is prohibited. New + clients must not use the insecure FLINK interface. + </para> + <para> + Besides dropping all modeset/global ioctls, render nodes also drop the + DRM-Master concept. There is no reason to associate render clients with + a DRM-Master as they are independent of any graphics server. Besides, + they must work without any running master, anyway. + Drivers must be able to run without a master object if they support + render nodes. If, on the other hand, a driver requires shared state + between clients which is visible to user-space and accessible beyond + open-file boundaries, they cannot support render nodes. + </para> + </sect1> + <!-- External: vblank handling -->
<sect1> diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c index 99fcd7c..abe5e7d 100644 --- a/drivers/gpu/drm/drm_drv.c +++ b/drivers/gpu/drm/drm_drv.c @@ -68,7 +68,7 @@ static const struct drm_ioctl_desc drm_ioctls[] = { DRM_IOCTL_DEF(DRM_IOCTL_GET_MAP, drm_getmap, DRM_UNLOCKED), DRM_IOCTL_DEF(DRM_IOCTL_GET_CLIENT, drm_getclient, DRM_UNLOCKED), DRM_IOCTL_DEF(DRM_IOCTL_GET_STATS, drm_getstats, DRM_UNLOCKED), - DRM_IOCTL_DEF(DRM_IOCTL_GET_CAP, drm_getcap, DRM_UNLOCKED), + DRM_IOCTL_DEF(DRM_IOCTL_GET_CAP, drm_getcap, DRM_UNLOCKED|DRM_RENDER_ALLOW), DRM_IOCTL_DEF(DRM_IOCTL_SET_VERSION, drm_setversion, DRM_MASTER),
DRM_IOCTL_DEF(DRM_IOCTL_SET_UNIQUE, drm_setunique, DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY), @@ -131,14 +131,14 @@ static const struct drm_ioctl_desc drm_ioctls[] = {
DRM_IOCTL_DEF(DRM_IOCTL_UPDATE_DRAW, drm_noop, DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY),
- DRM_IOCTL_DEF(DRM_IOCTL_GEM_CLOSE, drm_gem_close_ioctl, DRM_UNLOCKED), - DRM_IOCTL_DEF(DRM_IOCTL_GEM_FLINK, drm_gem_flink_ioctl, DRM_AUTH|DRM_UNLOCKED), + DRM_IOCTL_DEF(DRM_IOCTL_GEM_CLOSE, drm_gem_close_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW), + DRM_IOCTL_DEF(DRM_IOCTL_GEM_FLINK, drm_gem_flink_ioctl, DRM_AUTH|DRM_UNLOCKED|DRM_RENDER_ALLOW), DRM_IOCTL_DEF(DRM_IOCTL_GEM_OPEN, drm_gem_open_ioctl, DRM_AUTH|DRM_UNLOCKED),
DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETRESOURCES, drm_mode_getresources, DRM_CONTROL_ALLOW|DRM_UNLOCKED),
- DRM_IOCTL_DEF(DRM_IOCTL_PRIME_HANDLE_TO_FD, drm_prime_handle_to_fd_ioctl, DRM_AUTH|DRM_UNLOCKED), - DRM_IOCTL_DEF(DRM_IOCTL_PRIME_FD_TO_HANDLE, drm_prime_fd_to_handle_ioctl, DRM_AUTH|DRM_UNLOCKED), + DRM_IOCTL_DEF(DRM_IOCTL_PRIME_HANDLE_TO_FD, drm_prime_handle_to_fd_ioctl, DRM_AUTH|DRM_UNLOCKED|DRM_RENDER_ALLOW), + DRM_IOCTL_DEF(DRM_IOCTL_PRIME_FD_TO_HANDLE, drm_prime_fd_to_handle_ioctl, DRM_AUTH|DRM_UNLOCKED|DRM_RENDER_ALLOW),
DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETPLANERESOURCES, drm_mode_getplane_res, DRM_CONTROL_ALLOW|DRM_UNLOCKED), DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETCRTC, drm_mode_getcrtc, DRM_CONTROL_ALLOW|DRM_UNLOCKED), @@ -428,9 +428,10 @@ long drm_ioctl(struct file *filp, DRM_DEBUG("no function\n"); retcode = -EINVAL; } else if (((ioctl->flags & DRM_ROOT_ONLY) && !capable(CAP_SYS_ADMIN)) || - ((ioctl->flags & DRM_AUTH) && !file_priv->authenticated) || + ((ioctl->flags & DRM_AUTH) && !drm_is_render_client(file_priv) && !file_priv->authenticated) || ((ioctl->flags & DRM_MASTER) && !file_priv->is_master) || - (!(ioctl->flags & DRM_CONTROL_ALLOW) && (file_priv->minor->type == DRM_MINOR_CONTROL))) { + (!(ioctl->flags & DRM_CONTROL_ALLOW) && (file_priv->minor->type == DRM_MINOR_CONTROL)) || + (!(ioctl->flags & DRM_RENDER_ALLOW) && drm_is_render_client(file_priv))) { retcode = -EACCES; } else { if (cmd & (IOC_IN | IOC_OUT)) { diff --git a/drivers/gpu/drm/drm_fops.c b/drivers/gpu/drm/drm_fops.c index 3a24385..4f3bbe2 100644 --- a/drivers/gpu/drm/drm_fops.c +++ b/drivers/gpu/drm/drm_fops.c @@ -300,10 +300,10 @@ static int drm_open_helper(struct inode *inode, struct file *filp, goto out_prime_destroy; }
- - /* if there is no current master make this fd it */ + /* if there is no current master make this fd it, but do not create + * any master object for render clients */ mutex_lock(&dev->struct_mutex); - if (!priv->minor->master) { + if (!priv->minor->master && !drm_is_render_client(priv)) { /* create a new master */ priv->minor->master = drm_master_create(priv->minor); if (!priv->minor->master) { @@ -341,12 +341,11 @@ static int drm_open_helper(struct inode *inode, struct file *filp, goto out_close; } } - mutex_unlock(&dev->struct_mutex); - } else { + } else if (!drm_is_render_client(priv)) { /* get a reference to the master */ priv->master = drm_master_get(priv->minor->master); - mutex_unlock(&dev->struct_mutex); } + mutex_unlock(&dev->struct_mutex);
mutex_lock(&dev->struct_mutex); list_add(&priv->lhead, &dev->filelist); @@ -547,7 +546,8 @@ int drm_release(struct inode *inode, struct file *filp) iput(container_of(dev->dev_mapping, struct inode, i_data));
/* drop the reference held my the file priv */ - drm_master_put(&file_priv->master); + if (file_priv->master) + drm_master_put(&file_priv->master); file_priv->is_master = 0; list_del(&file_priv->lhead); mutex_unlock(&dev->struct_mutex); diff --git a/drivers/gpu/drm/drm_pci.c b/drivers/gpu/drm/drm_pci.c index 80c0b2b..e713f0a 100644 --- a/drivers/gpu/drm/drm_pci.c +++ b/drivers/gpu/drm/drm_pci.c @@ -348,6 +348,12 @@ int drm_get_pci_dev(struct pci_dev *pdev, const struct pci_device_id *ent, goto err_g2; }
+ if (drm_core_check_feature(dev, DRIVER_RENDER)) { + ret = drm_get_minor(dev, &dev->render, DRM_MINOR_RENDER); + if (ret) + goto err_g21; + } + if ((ret = drm_get_minor(dev, &dev->primary, DRM_MINOR_LEGACY))) goto err_g3;
@@ -377,6 +383,9 @@ int drm_get_pci_dev(struct pci_dev *pdev, const struct pci_device_id *ent, err_g4: drm_put_minor(&dev->primary); err_g3: + if (drm_core_check_feature(dev, DRIVER_RENDER)) + drm_put_minor(&dev->render); +err_g21: if (drm_core_check_feature(dev, DRIVER_MODESET)) drm_put_minor(&dev->control); err_g2: diff --git a/drivers/gpu/drm/drm_platform.c b/drivers/gpu/drm/drm_platform.c index b8a282e..32e15f0 100644 --- a/drivers/gpu/drm/drm_platform.c +++ b/drivers/gpu/drm/drm_platform.c @@ -69,6 +69,12 @@ int drm_get_platform_dev(struct platform_device *platdev, goto err_g1; }
+ if (drm_core_check_feature(dev, DRIVER_RENDER)) { + ret = drm_get_minor(dev, &dev->render, DRM_MINOR_RENDER); + if (ret) + goto err_g11; + } + ret = drm_get_minor(dev, &dev->primary, DRM_MINOR_LEGACY); if (ret) goto err_g2; @@ -100,6 +106,9 @@ int drm_get_platform_dev(struct platform_device *platdev, err_g3: drm_put_minor(&dev->primary); err_g2: + if (drm_core_check_feature(dev, DRIVER_RENDER)) + drm_put_minor(&dev->render); +err_g11: if (drm_core_check_feature(dev, DRIVER_MODESET)) drm_put_minor(&dev->control); err_g1: diff --git a/drivers/gpu/drm/drm_stub.c b/drivers/gpu/drm/drm_stub.c index d372e27..b785a8e 100644 --- a/drivers/gpu/drm/drm_stub.c +++ b/drivers/gpu/drm/drm_stub.c @@ -473,6 +473,9 @@ void drm_put_dev(struct drm_device *dev) if (drm_core_check_feature(dev, DRIVER_MODESET)) drm_put_minor(&dev->control);
+ if (drm_core_check_feature(dev, DRIVER_RENDER)) + drm_put_minor(&dev->render); + if (driver->driver_features & DRIVER_GEM) drm_gem_destroy(dev);
@@ -489,6 +492,8 @@ void drm_unplug_dev(struct drm_device *dev) /* for a USB device */ if (drm_core_check_feature(dev, DRIVER_MODESET)) drm_unplug_minor(dev->control); + if (drm_core_check_feature(dev, DRIVER_RENDER)) + drm_unplug_minor(dev->render); drm_unplug_minor(dev->primary);
mutex_lock(&drm_global_mutex); diff --git a/drivers/gpu/drm/drm_usb.c b/drivers/gpu/drm/drm_usb.c index 34a156f..9755fea 100644 --- a/drivers/gpu/drm/drm_usb.c +++ b/drivers/gpu/drm/drm_usb.c @@ -33,6 +33,12 @@ int drm_get_usb_dev(struct usb_interface *interface, if (ret) goto err_g1;
+ if (drm_core_check_feature(dev, DRIVER_RENDER)) { + ret = drm_get_minor(dev, &dev->render, DRM_MINOR_RENDER); + if (ret) + goto err_g11; + } + ret = drm_get_minor(dev, &dev->primary, DRM_MINOR_LEGACY); if (ret) goto err_g2; @@ -62,6 +68,9 @@ int drm_get_usb_dev(struct usb_interface *interface, err_g3: drm_put_minor(&dev->primary); err_g2: + if (drm_core_check_feature(dev, DRIVER_RENDER)) + drm_put_minor(&dev->render); +err_g11: drm_put_minor(&dev->control); err_g1: kfree(dev); diff --git a/include/drm/drmP.h b/include/drm/drmP.h index 6544450..3ef63df 100644 --- a/include/drm/drmP.h +++ b/include/drm/drmP.h @@ -154,6 +154,7 @@ int drm_err(const char *func, const char *format, ...); #define DRIVER_GEM 0x1000 #define DRIVER_MODESET 0x2000 #define DRIVER_PRIME 0x4000 +#define DRIVER_RENDER 0x8000
#define DRIVER_BUS_PCI 0x1 #define DRIVER_BUS_PLATFORM 0x2 @@ -308,6 +309,7 @@ typedef int drm_ioctl_compat_t(struct file *filp, unsigned int cmd, #define DRM_ROOT_ONLY 0x4 #define DRM_CONTROL_ALLOW 0x8 #define DRM_UNLOCKED 0x10 +#define DRM_RENDER_ALLOW 0x20
struct drm_ioctl_desc { unsigned int cmd; @@ -1204,6 +1206,7 @@ struct drm_device { unsigned int agp_buffer_token; struct drm_minor *control; /**< Control node for card */ struct drm_minor *primary; /**< render type primary screen head */ + struct drm_minor *render; /**< render node for card */
struct drm_mode_config mode_config; /**< Current mode config */
@@ -1269,6 +1272,11 @@ static inline bool drm_modeset_is_locked(struct drm_device *dev) return mutex_is_locked(&dev->mode_config.mutex); }
+static inline bool drm_is_render_client(struct drm_file *file_priv) +{ + return file_priv->minor->type == DRM_MINOR_RENDER; +} + /******************************************************************/ /** \name Internal function definitions */ /*@{*/
On 07/07/2013 19:17, David Herrmann wrote:
Hi
This is v2 of the unified VMA offset manager series. The first draft is available at LWN [1]. This series replaces the VMA offset managers in GEM and TTM with a unified implementation.
The first 4 patches contain the new VMA offset manager and are the only patches that I propose for mainline inclusion. Patches 5-8 clean up drm_mm and are informational-only. Daniel has similar patches in i915-next and I will rebase them once it is merged. Ignore them if you're not interested. Patches 9-19 implement mmap access control. Comments are welcome! They are intended for mainline inclusion, too, but probably require some more review rounds. I'd really appreciate if driver authors could comment on the implementation. Patch 20 shows the main motivation behind this whole series: render nodes. It is marked RFC and I will resend once the VMA manager is merged upstream. Feel free to test it.
One more note regarding DRM-Master: Render-clients are independent of DRM-Master (see the DocBook documentation). It really doesn't make sense to create/bind a DRM-Master object to render-clients. However, a lot of core DRM code depends on ->master != NULL. Drivers need to take care not to call into those core modules if they implement DRIVER_RENDER. I plan on removing multiple-master-support in the next series. So there will be only one global master and each open-file is bound to it. This will make it very easy to phase out "drm_master". The planned "modeset" nodes provide a nice replacement. If anyone knows a **currently used** use-case for multiple-masters, please tell me. I couldn't find one that _actually works_. (SetMaster+DropMaster will obviously stay functional even with drm_master removed).
(series available at: https://github.com/dvdhrm/linux/tree/rnodes)
Comments welcome! Cheers David
Hi David,
Thanks for this patchset. I am away from my computers for testing but I was wondering if you based your vma rework on Dave's implementation. If so, you may have the same bug that I had with it.
I cannot remember the details of the bug, but I could trigger it by rebooting kde around 13 times on radeon. I didn't have this problem with Nouveau.
I am not competent to judge this work but I will try to test it and check with my security tests that I wrote that the problem is indeed solved on nouveau and radeon.
Looking forward to seeing your proposition for the userspace :)
Cheers, Martin
Hi
On Thu, Jul 11, 2013 at 1:12 PM, Martin Peres martin.peres@labri.fr wrote:
On 07/07/2013 19:17, David Herrmann wrote:
Hi
This is v2 of the unified VMA offset manager series. The first draft is available at LWN [1]. This series replaces the VMA offset managers in GEM and TTM with a unified implementation.
The first 4 patches contain the new VMA offset manager and are the only patches that I propose for mainline inclusion. Patches 5-8 clean up drm_mm and are informational-only. Daniel has similar patches in i915-next and I will rebase them once it is merged. Ignore them if you're not interested. Patches 9-19 implement mmap access control. Comments are welcome! They are intended for mainline inclusion, too, but probably require some more review rounds. I'd really appreciate if driver authors could comment on the implementation. Patch 20 shows the main motivation behind this whole series: render nodes. It is marked RFC and I will resend once the VMA manager is merged upstream. Feel free to test it.
One more note regarding DRM-Master: Render-clients are independent of DRM-Master (see the DocBook documentation). It really doesn't make sense to create/bind a DRM-Master object to render-clients. However, a lot of core DRM code depends on ->master != NULL. Drivers need to take care not to call into those core modules if they implement DRIVER_RENDER. I plan on removing multiple-master-support in the next series. So there will be only one global master and each open-file is bound to it. This will make it very easy to phase out "drm_master". The planned "modeset" nodes provide a nice replacement. If anyone knows a **currently used** use-case for multiple-masters, please tell me. I couldn't find one that _actually works_. (SetMaster+DropMaster will obviously stay functional even with drm_master removed).
(series available at: https://github.com/dvdhrm/linux/tree/rnodes)
Comments welcome! Cheers David
Hi David,
Thanks for this patchset. I am away from my computers for testing but I was wondering if you based your vma rework on Dave's implementation. If so, you may have the same bug that I had with it.
I cannot remember the details of the bug, but I could trigger it by rebooting kde around 13 times on radeon. I didn't have this problem with Nouveau.
It is based on Dave's code, but I rewrote all of it and fixed several bugs. For instance, there was a TTM ref-count leak for BOs in TTM core and a TTM locking issue. I didn't encounter any bugs so far, but I didn't try to restart the xserver 13 times. I will do some more stress-tests, but it is quite likely you hit one of those two bugs with radeon.
I am not competent to judge this work but I will try to test it and check with my security tests that I wrote that the problem is indeed solved on nouveau and radeon.
The changes are actually quite simple. I intentionally kept TTM locking as it was before, even though I think we could reduce it now. Anyway, I will resend v3 (which now includes tegra and staging) this weekend. Hopefully we can get it into linux-next soon.
Thanks! David
Looking forward to seeing your proposition for the userspace :)
Cheers, Martin
On 11/07/2013 13:21, David Herrmann wrote:
Hi
On Thu, Jul 11, 2013 at 1:12 PM, Martin Peres martin.peres@labri.fr wrote:
On 07/07/2013 19:17, David Herrmann wrote:
Hi
This is v2 of the unified VMA offset manager series. The first draft is available at LWN [1]. This series replaces the VMA offset managers in GEM and TTM with a unified implementation.
The first 4 patches contain the new VMA offset manager and are the only patches that I propose for mainline inclusion. Patches 5-8 clean up drm_mm and are informational-only. Daniel has similar patches in i915-next and I will rebase them once it is merged. Ignore them if you're not interested. Patches 9-19 implement mmap access control. Comments are welcome! They are intended for mainline inclusion, too, but probably require some more review rounds. I'd really appreciate if driver authors could comment on the implementation. Patch 20 shows the main motivation behind this whole series: render nodes. It is marked RFC and I will resend once the VMA manager is merged upstream. Feel free to test it.
One more note regarding DRM-Master: Render-clients are independent of DRM-Master (see the DocBook documentation). It really doesn't make sense to create/bind a DRM-Master object to render-clients. However, a lot of core DRM code depends on ->master != NULL. Drivers need to take care not to call into those core modules if they implement DRIVER_RENDER. I plan on removing multiple-master-support in the next series. So there will be only one global master and each open-file is bound to it. This will make it very easy to phase out "drm_master". The planned "modeset" nodes provide a nice replacement. If anyone knows a **currently used** use-case for multiple-masters, please tell me. I couldn't find one that _actually works_. (SetMaster+DropMaster will obviously stay functional even with drm_master removed).
(series available at: https://github.com/dvdhrm/linux/tree/rnodes)
Comments welcome! Cheers David
Hi David,
Thanks for this patchset. I am away from my computers for testing but I was wondering if you based your vma rework on Dave's implementation. If so, you may have the same bug that I had with it.
I cannot remember the details of the bug, but I could trigger it by rebooting kde around 13 times on radeon. I didn't have this problem with Nouveau.
It is based on Dave's code, but I rewrote all of it and fixed several bugs. For instance, there was a TTM ref-count leak for BOs in TTM core and a TTM locking issue. I didn't encounter any bugs so far, but I didn't try to restart the xserver 13 times. I will do some more stress-tests, but it is quite likely you hit one of those two bugs with radeon.
Yeah, the problem I had was related to ref-count and I was trying to fix the locking too.
I am not competent to judge this work but I will try to test it and check with my security tests that I wrote that the problem is indeed solved on nouveau and radeon.
The changes are actually quite simple. I intentionally kept TTM locking as it was before, even though I think we could reduce it now. Anyway, I will resend v3 (which now includes tegra and staging) this weekend. Hopefully we can get it into linux-next soon.
Very nice! Looking forward to it.
dri-devel@lists.freedesktop.org
-
David Herrmann -
Martin Peres